Skip to content

refactor: don't load repo-phase objects from build-phase#2056

Merged
rickeylev merged 1 commit intobazel-contrib:mainfrom
rickeylev:split.pypi.repo.build.refs
Jul 11, 2024
Merged

refactor: don't load repo-phase objects from build-phase#2056
rickeylev merged 1 commit intobazel-contrib:mainfrom
rickeylev:split.pypi.repo.build.refs

Conversation

@rickeylev
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev commented Jul 11, 2024

As a general practice, the repo-phase and build-phase shouldn't load code from one
another because they can't use each other's objects. It can also result in confusing
behavior because the "starlark environment" is slightly different between the two phases.

Additionally, Google's version of Bazel essentially disables repo-phase objects, so
loading e.g. http_archive results in errors. This makes it more difficult to import
rules_python into Google, as we have to maintain patches to cut out the code (and thus
we spend more time trying to import the code than working on it).

@rickeylev rickeylev force-pushed the split.pypi.repo.build.refs branch from eeabd9f to 6a9fa14 Compare July 11, 2024 02:55
@rickeylev rickeylev force-pushed the split.pypi.repo.build.refs branch from 6a9fa14 to 53a42e8 Compare July 11, 2024 03:23
@rickeylev rickeylev changed the title wip: factoring out build-phase loading repo-phase symbols refactor: don't load repo-phase objects from build-phase Jul 11, 2024
@rickeylev rickeylev marked this pull request as ready for review July 11, 2024 03:55
@rickeylev rickeylev requested review from aignas and groodt as code owners July 11, 2024 03:55
Copy link
Copy Markdown
Collaborator

@groodt groodt left a comment

Choose a reason for hiding this comment

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

LGTM

At canva we also ban the usage of these macros in our repo :) I'd be happy to see them gone entirely, but it doesn't seem viable at this stage because there is too much entrenched usage.

@rickeylev rickeylev added this pull request to the merge queue Jul 11, 2024
Merged via the queue into bazel-contrib:main with commit 03854a2 Jul 11, 2024
@rickeylev rickeylev deleted the split.pypi.repo.build.refs branch July 11, 2024 21:06
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