Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Don't parse stub file if interpreter is explicitly provided #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dflems
Copy link

@dflems dflems commented Sep 8, 2020

It seems like --stub_file is only parsed to derive which interpreter is being used. Correct me if I'm wrong, but it seems like passing in --interpreter should short-circuit parsing that file. Seems like this will allow for using in-workspace py_runtime definitions as long as the interpreter is specified explicitly in the parfile() rule.

I might be missing some important context though.

@oxidase
Copy link

oxidase commented Sep 15, 2020

I had the same issue with non-system Python runtimes as parsing stubs before checking args.interpreter exits with an exception. I'll dismiss my PR #129 as this PR also add interpreter to the attributes list that is better than my workaround compiler_args = ["--interpreter", "/usr/bin/python3"],

@brandjon please could you review the fix.

subpar.bzl Show resolved Hide resolved
@dflems
Copy link
Author

dflems commented Sep 16, 2020

closing and reopening to kickstart CI (which didn't seem to pick up my last commit)

@dflems dflems closed this Sep 16, 2020
@dflems dflems reopened this Sep 16, 2020
@dflems dflems closed this Sep 22, 2020
@dflems dflems reopened this Sep 22, 2020
@dflems
Copy link
Author

dflems commented Oct 28, 2020

@brandjon 👋 does anyone pay attention to this repo? 😢


if args.interpreter:
interpreter = args.interpreter
interpreter = args.interpreter or parse_stub(args.stub_file)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note parse_stub also validates interpreter. Really it should be broken into parse_interpreter and validate_interpreter
then this would be:

interpreter = args.interpreter or parse_interpreter(args.stub_file)
validate_interperter(interpreter)

or something

@thundergolfer
Copy link

thundergolfer commented May 25, 2021

Seems like this will allow for using in-workspace py_runtime definitions as long as the interpreter is specified explicitly in the parfile() rule.

If you have an in-workspace interpreter at label @python3//:bin/python3 won't passing this as interpreter value result in a .par with a shebang like:

#!@python3//:bin/python3

How does the in-workspace interpreter label get translated into an absolute filepath?

@dflems
Copy link
Author

dflems commented Jul 21, 2021

@thundergolfer Ah that's true... On my end, I'm setting interpreter to /usr/bin/env python3 as we have some par_binarys that we export as internal tools

@bshashank bshashank mentioned this pull request Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants