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

Revisit --enable_runfiles #9248

Closed
Tracked by #12665
buchgr opened this issue Aug 26, 2019 · 7 comments
Closed
Tracked by #12665

Revisit --enable_runfiles #9248

buchgr opened this issue Aug 26, 2019 · 7 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request

Comments

@buchgr
Copy link
Contributor

buchgr commented Aug 26, 2019

The flags --build_runfile_manifests and --build_runfile_links govern whether runfile manifests and runfile trees should be created at the time of building a binary. bazel run/test has extra logic to create runfile trees on-demand if --nobuild_runfile_links is set. This is where --enable_runfiles comes in. It disables any runfile trees creation and instead only copies the input manifest to the output manifest.

  1. I suggest we deprecate and remove the flag.
  2. Instead we introduce --test_runfile_links which enables/disables the symlink tree creation for bazel run/test. On Windows, both --build_runfile_links and --test_runfile_links would be disabled by default.
  3. In case of --nobuild_runfile_links it also copies the input manifest to the output manifest. This is unnecessary and can be slow.
  4. It has a correctness issue in that if its value changes analysis is not rerun: Runfiles tree is not recreated when value of --enable_runfiles changes #9150

@meteorcloudy @laszlocsomor

@iirina iirina added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website untriaged labels Aug 28, 2019
@nicolov
Copy link
Contributor

nicolov commented Sep 2, 2019

Just to add to the list, --enable_runfiles=false doesn't seem to work with bazel test, the symlink tree is created anyway: #9164

@philwo philwo added team-Remote-Exec Issues and PRs for the Execution (Remote) team and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Sep 3, 2019
@laszlocsomor
Copy link
Contributor

Sounds like --[no]test_runfile_links means the same as --[no]enable_runfiles, is that correct?

@buchgr
Copy link
Contributor Author

buchgr commented Sep 6, 2019

@laszlocsomor no. --[no]build_runfile_links and --[no]test_runfile_links combined are the same as --[no]enable_runfiles

@laszlocsomor
Copy link
Contributor

Ah, gotcha. They individually control runfiles link generation in build phase vs. test phase.

@coeuvre coeuvre added P3 We're not considering working on this, but happy to review a PR. (No assignee) type: feature request and removed untriaged labels Dec 9, 2020
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 15, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@tjgq tjgq reopened this Jun 21, 2023
@tjgq
Copy link
Contributor

tjgq commented Jun 21, 2023

Still relevant.

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Jun 22, 2023
@coeuvre
Copy link
Member

coeuvre commented Sep 6, 2023

We have made the conclusion in #18580, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants