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

http_archive doesn't work well for Windows #7458

Closed
iirina opened this issue Feb 18, 2019 · 17 comments
Closed

http_archive doesn't work well for Windows #7458

iirina opened this issue Feb 18, 2019 · 17 comments
Labels
area-Windows Windows-specific issues and feature requests P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) platform: windows stale Issues or PRs that are stale (no activity for 30 days) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug

Comments

@iirina
Copy link
Contributor

iirina commented Feb 18, 2019

The following scenario is not well supported on Windows:

  • declare an http_archive which contains top-level files
  • one of the top-level files in the http_archvie is invoked by a bazel command

The bazel command only succeeds if it's a clean build, otherwise fails with an error similar to:

ERROR: Source forest creation failed: D:/b/sdlshnzi/execroot/io_bazel/_tmp/2713b7a92d8298dcf32c3b44de75c8b1/root/sb26b24g/execroot/main/external/remote_java_tools/JavaBuilder_deploy.jar (Permission denied)

IIUC this is because Bazel tries to remove all non-symlink directories/files from the execroot tree. On Windows there are no symlinks for regular files and bazel uses copies of the original files, so bazel tries to remove all the files downloaded from http_archive. If they are already in use by some process (this happens if they were already invoked/used by previous bazel runs) bazel fails to delete them.

This issue is a generalization of #7440, which was fixed by placing all top-level files under a directory in the http_archive.

@iirina
Copy link
Contributor Author

iirina commented Feb 18, 2019

cc @laszlocsomor @lberki

@meteorcloudy meteorcloudy added type: bug category: extensibility > external repositories platform: windows area-Windows Windows-specific issues and feature requests P2 We'll consider working on this in future. (Assignee optional) labels Feb 19, 2019
@meteorcloudy
Copy link
Member

@laszlocsomor Since you are working on new_local_repository already, can you also look into this?

@meteorcloudy
Copy link
Member

Is there an easy way to put JavaBuilder_deploy.jar under a sub-directory?
This issue is also causing Bazelisk + Incompatible flags to fail on Windows
https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/45#4b55ba23-99ba-49c0-9cd6-9dee480c51ad

@laszlocsomor
Copy link
Contributor

@meteorcloudy : I'm confused -- that build is green.

@meteorcloudy
Copy link
Member

@laszlocsomor When running with Bazelisk, the job will continue and be green (because we expect some failures when using incompatible flags). But if check the Running Bazel with no incompatible flags section at the end of the log, you'll see the build failed with this error. And because of this, the pipeline listed Bazel on Windows as an already failing job.

@laszlocsomor
Copy link
Contributor

@iirina : How is the //external:remote_java_tools repository created? With 0.23.2, I get:

C:\src\bazel>bazel query //external:remote_java_tools --output=build
# /DEFAULT.WORKSPACE.SUFFIX:208:1
http_archive(
  name = "remote_java_tools",
  urls = ["https://mirror.bazel.build/bazel_java_tools/java_tools_pkg-0.1.tar.gz"],
  sha256 = "df33ddb3054f0ee70389368bd1dc2efe72eeb1f489dbcdf948f3f3b3058646b7",
  build_file = "@bazel_tools//tools/jdk:BUILD.pkg",
)

Contents of java_tools_pkg-0.1.tar.gz:

  $ tar tf java_tools_pkg-0.1.tar.gz | grep JavaBuilder_deploy.jar
./JavaBuilder_deploy.jar
./VanillaJavaBuilder_deploy.jar

We need to repackage this tar and move the .jar files to subdirectories, then update jdk.WORKSPACE, then release Bazel. Is that correct?

@iirina
Copy link
Contributor Author

iirina commented Mar 15, 2019

remote_java_tools comes from the http_archive you mentioned. If you want to update the java tools version then yes the archive has to be repackaged, uploaded to GCS and jdk.WORKSPACE needs to be updated before a Bazel release.

We released the java tools several times after 0.1 (I'm surprised that bazel 0.23 was released with the first version). The java tools versions >= 0.4 have all their content under a java_tools top level directory and this issue is not present anymore. If we want to do a patch release we can just update jdk.WORKSPACE to point to the latest java tools version (I can help with that).

@meteorcloudy
Copy link
Member

I don't think this is a regression, so maybe a patch release is not necessary?
But we do want to fix this before flipping --incompatible_use_toolchain_providers_in_java_common

@iirina
Copy link
Contributor Author

iirina commented Mar 15, 2019

@meteorcloudy Great, then it is already fixed. :)

@iirina
Copy link
Contributor Author

iirina commented Apr 24, 2019

@meteorcloudy @laszlocsomor any news on this issue? I'm thinking of adding a top-level WORKSPACE file to my http_archive, which will make it fail on Windows.

@laszlocsomor
Copy link
Contributor

No updates.
But I don't think a WORKSPACE file is problematic: no process will hold it open.
The problem is with files that we run as subprocesses or otherwise hold open handles to.

@iirina
Copy link
Contributor Author

iirina commented Apr 24, 2019

Oh, that's great! This applies to BUILD files as well, IIUC?

@laszlocsomor
Copy link
Contributor

Yes it does. If no subprocess holds this file open, you're fine. (Jar files and data files of persistent workers are risky.)

@iirina
Copy link
Contributor Author

iirina commented Apr 24, 2019

Thanks!

@laszlocsomor
Copy link
Contributor

@iirina : does this bug still matter? It's worth documenting that no file from the root of an archive should be used in a worker, but is there anything beyond that?

@iirina
Copy link
Contributor Author

iirina commented Jul 5, 2019

I'll leave it up to the Windows team to decide on the priority/fix. The java rules are not affected by this issue anymore, so from this point of view it doesn't matter. Documenting this behavior is definitely helpful. Depending on what type of archives the Bazel users have this may or may not be an issue in the future.

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@meteorcloudy meteorcloudy added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Feb 23, 2022
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 16, 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.

@sgowroji sgowroji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) platform: windows stale Issues or PRs that are stale (no activity for 30 days) team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug
Projects
None yet
Development

No branches or pull requests

6 participants