Skip to content

Exclude in-repo python from pexes#218

Merged
peterebden merged 3 commits intoplease-build:masterfrom
peterebden:deal-with-in-repo-python
Nov 13, 2024
Merged

Exclude in-repo python from pexes#218
peterebden merged 3 commits intoplease-build:masterfrom
peterebden:deal-with-in-repo-python

Conversation

@peterebden
Copy link
Contributor

This is a bit awkward. An in-repo interpreter will probably look something like //third_party/cpython:cpython|python, which expands to TOOLS_PYTHON=third_party/cpython/usr/bin/python3 or whatever when using remote execution (when all tools are within the working path). This means that it excludes just the executable and not any of the rest of it, which is huge and we don't want it in everything.

This is a slightly grungy way of telling it to cut that out, assuming that we don't want to include any part of the interpreter if it's an annotated label.

I've added a test, which failed before and passes for me. It won't do an awful lot on GHA though since we don't have a remote execution setup for this repo.

cmd = ' && '.join([compile_cmd, 'rm -f $SRCS_SRCS', cmd])
else:
cmd = ' && '.join([compile_cmd, cmd])
if looks_like_build_label(interpreter) and ('|' in interpreter):
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible use case for an is_named_output built-in here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe so, generally things aren't meant to care but this is one case where it turns out we do...
Might also want it to be able to do this decomposition - this should be fine but I prefer not to encode structural knowledge like this into random build rules

@peterebden peterebden merged commit bd2b13d into please-build:master Nov 13, 2024
@peterebden peterebden deleted the deal-with-in-repo-python branch November 13, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants