Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rules): add build_data_file field to PyExecutableInfo #2181

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

oprypin
Copy link
Contributor

@oprypin oprypin commented Sep 4, 2024

PyExecutableInfo was added in #2166 with the field runfiles_without_exe that intentionally excludes files that are specific to
that target/executable, such as the build data file (which may contain the target name,
or other target-specific information).

However, consuming tools (such as ones used within Google) may need to derive a file from
that build data, override it completely, or be happy with its content as is. To aid that
case, expose it via PyExecutableInfo.

PyExecutableInfo was added in #2166 with the field `runfiles_without_exe` that intentionally excludes unrelated files. But, within Google we found that we need to pick up the file with build stamp information at that stage as well, so it is being additionally exposed.
Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Just the small defense-in-depth type of change.

python/private/py_executable_info.bzl Outdated Show resolved Hide resolved
python/private/common/py_executable.bzl Outdated Show resolved Hide resolved
@rickeylev rickeylev added this pull request to the merge queue Sep 6, 2024
Merged via the queue into main with commit a507673 Sep 6, 2024
7 checks passed
@oprypin oprypin deleted the executable_builddatafile branch September 6, 2024 06:19
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