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

incompatible_merge_genfiles_directory: unify bazel-bin and bazel-genfiles directories #6761

Closed
laurentlb opened this issue Nov 26, 2018 · 7 comments
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)

Comments

@laurentlb
Copy link
Contributor

We plan to unify the bazel-bin and bazel-genfiles directories.

This means that bazel-genfiles will point to the same location as bazel-bin, since the distinction was not meaningful. This will allow us to make simplifications in Bazel and its API.

See discussion: https://groups.google.com/d/msg/bazel-dev/5iq7n1soenA/xGM_M1NOCAAJ

@laurentlb laurentlb added P1 I'll work on this now. (Assignee required) team-Starlark incompatible-change Incompatible/breaking change labels Nov 26, 2018
bazel-io pushed a commit that referenced this issue Nov 26, 2018
…atible_merge_genfiles_directory`

#6761

RELNOTES:
  New incompatible flag --incompatible_merge_genfiles_directory
PiperOrigin-RevId: 222826123
@laurentlb laurentlb changed the title --incompatible_merge_genfiles_directory incompatible_merge_genfiles_directory: unify bazel-bin and bazel-genfiles directories Jan 7, 2019
@laurentlb
Copy link
Contributor Author

One downstream project fails with--incompatible_merge_genfiles_directory: protobuf

https://buildkite.com/bazel/bazel-at-release-plus-incompatible-flags/builds/56#70fd5567-2b59-4012-be62-1b082da936b0

@laurentlb
Copy link
Contributor Author

Still blocked by Protobuf (protocolbuffers/protobuf#5691).

@meteorcloudy
Copy link
Member

@laurentlb FYI, Bazelisk + Incompatible flags couldn't catch this breakage. Because building with this flag seems have an impact on subsequent builds.
See https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/18#d6d93bbf-ca8a-432f-bd99-23d93577ede5
Unless we do a bazel clean between the incompatible flags and no incompatible flags build, we still get the same error message.

cloud-robotics-github-robot pushed a commit to googlecloudrobotics/core that referenced this issue Mar 5, 2019
For historical reasons, Bazel has two directories for generated files,
although a future version of Bazel will remove the genfiles directory:
bazelbuild/bazel#6761

Currently, rosmsg() copies all the source files (*.msg, *.srv) to the
output directory, so that the directory structure matches the needs of
ROS's codegen tools. It uses a hacky way of finding the paths to the
transitive dependencies within the output directory, which includes a
direct reference to genfiles. By using output_to_bindir=True, we can
avoid the direct reference.

Fixing the hack would involve rewriting the rosmsg() macro to use a
rule, which wouldn't be too hard but might break compatibility.

See #2.

Change-Id: Ibae3451e1329e397329089aa6796c5477789c788
GitOrigin-RevId: e38ef5d
@jin
Copy link
Member

jin commented Mar 27, 2019

This will cause all of these scripts / code that assume existence of bazel-genfiles to stop working, right? https://github.com/search?q=bazel-genfiles&type=Code

We can catch possible breakages due to this change using Buildkite iff the bazel-genfiles path is referenced within the build. However, many of the references are from configuration files and external scripts. How we can improve our imcompatible change infra to work with this kind of changes?

I'm just wondering about the user experience of getting this in the next release, if some part of the bazel-external infrastructure breaks because the genfiles dir no longer exists, and I had no warning about it when using previous Bazel versions interactively.

@laurentlb
Copy link
Contributor Author

When testing downstream projects, we noticed very few issues. But it's true that it tests only within Bazel. Scripts that call Bazel and then look at the specific directory might break.

Not sure what we can do, apart from advertising the change more loudly.
(at least, it shows why this should be done before Bazel 1.0)

@laurentlb
Copy link
Contributor Author

laurentlb commented Mar 27, 2019

Projects that rely on bazel-genfiles can:

  • Either specify the flag --incompatible_merge_genfiles_directory=false. This flag will be available for a long time (even after Bazel 1.0), to give users more time to migrate.
  • Or create a symlink: ln -s bazel-bin bazel-genfiles

@jin
Copy link
Member

jin commented Mar 27, 2019

When testing downstream projects, we noticed very few issues. But it's true that it tests only within Bazel. Scripts that call Bazel and then look at the specific directory might break.

The symlinks are, in a way, the only user facing filesystem level API that Bazel exposes. I also have no idea what else we can do. The Downstream pipeline has no visibility into the bazel-external build infra of our users :(

advertising the change more loudly. (at least, it shows why this should be done before Bazel 1.0)

Completely agree.

swiple-rules-gardener pushed a commit to bazelbuild/rules_swift that referenced this issue Aug 23, 2019
…ed by ClangImporter.

genfiles_dir is already searched but is deprecated (bazelbuild/bazel#6761) and the C rules have been adding bin_dir to their quote include paths since June 2018, so we should catch up.

RELNOTES: Swift modules that import C modules can now find headers located in the bazel-bin directory.
PiperOrigin-RevId: 264958475
swiple-rules-gardener pushed a commit to bazelbuild/rules_swift that referenced this issue Aug 23, 2019
…ed by ClangImporter.

genfiles_dir is already searched but is deprecated (bazelbuild/bazel#6761) and the C rules have been adding bin_dir to their quote include paths since June 2018, so we should catch up.

RELNOTES: Swift modules that import C modules can now find headers located in the bazel-bin directory.
PiperOrigin-RevId: 264958475
swiple-rules-gardener pushed a commit to bazelbuild/rules_swift that referenced this issue Aug 23, 2019
…ed by ClangImporter.

genfiles_dir is already searched but is deprecated (bazelbuild/bazel#6761) and the C rules have been adding bin_dir to their quote include paths since June 2018, so we should catch up.

RELNOTES: Swift modules that import C modules can now find headers located in the bazel-bin directory.
PiperOrigin-RevId: 265051482
serathius added a commit to serathius/kubernetes that referenced this issue Oct 25, 2019
Check both "bazel-genfiles" and "bazel-bin" due bazelbuild/bazel#6761
mako-github-bot pushed a commit to google/mako that referenced this issue Dec 2, 2019
…zel-bin/' instead of 'bazel-genfiles/'

These directories are being unified (bazelbuild/bazel#6761), so the distinction is not meaningful, except that in earlier versions of Bazel (e.g. 0.24ish) only bazel-bin is written to.

PiperOrigin-RevId: 282721127
Change-Id: I4028f0bb7ed93314c15fb3761762a30c5d647b42
fpnuseis added a commit to fpnuseis/bazel-cmakelists that referenced this issue Feb 12, 2020
keith added a commit to keith/bazel that referenced this issue Jul 19, 2022
keith added a commit to keith/bazel that referenced this issue Jul 20, 2022
keith added a commit to keith/bazel that referenced this issue Oct 11, 2022
keith added a commit to keith/bazel that referenced this issue Oct 11, 2022
keith added a commit to keith/bazel that referenced this issue Oct 11, 2022
keith added a commit to keith/bazel that referenced this issue Oct 11, 2022
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this issue Jul 19, 2023
…ed by ClangImporter.

genfiles_dir is already searched but is deprecated (bazelbuild/bazel#6761) and the C rules have been adding bin_dir to their quote include paths since June 2018, so we should catch up.

RELNOTES: Swift modules that import C modules can now find headers located in the bazel-bin directory.
PiperOrigin-RevId: 265051482
ClusterH pushed a commit to ClusterH/tensorflow_source that referenced this issue May 22, 2024
This makes path computation code shorter and more robust. In particular, it makes things work under Bazel flag --incompatible_merge_genfiles_directory (bazelbuild/bazel#6761).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible-change Incompatible/breaking change P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

4 participants