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

bazel_7: 7.4.0 -> 7.4.1 #355723

Merged
merged 4 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,19 @@
# Allow to independently override the jdks used to build and run respectively
buildJdk,
runJdk,
# Toggle for hacks for running bazel under buildBazelPackage:
# Always assume all markers valid (this is needed because we remove markers; they are non-deterministic).
# Also, don't clean up environment variables (so that NIX_ environment variables are passed to compilers).
enableNixHacks ? false,
version ? "7.4.0",
version ? "7.4.1",
}:

let
sourceRoot = ".";

src = fetchurl {
url = "https://github.com/bazelbuild/bazel/releases/download/${version}/bazel-${version}-dist.zip";
hash = "sha256-GY1wu3O5O7K2MMJv6wjE+DLnUgwjkHdmcqhT1o9G9Cg=";
hash = "sha256-gzhmGLxIn02jYmbvJiDsZKUmxobPBwQTMsr/fJU6+vU=";
};

defaultShellUtils =
Expand Down Expand Up @@ -112,23 +113,23 @@ let
if stdenv.hostPlatform.system == "x86_64-linux" then
fetchurl {
url = "https://github.com/bazelbuild/bazel/releases/download/${version}/bazel_nojdk-${version}-linux-x86_64";
hash = "sha256-0glQLNAU0aT7+3Hzv0+IzgvJlfs7y8wflEwFssIvnkk=";
hash = "sha256-CYL1paAtzTbfl7TfsqwJry/dkoTO/yZdHrX0NSA1+Ig=";
}
else if stdenv.hostPlatform.system == "aarch64-linux" then
fetchurl {
url = "https://github.com/bazelbuild/bazel/releases/download/${version}/bazel_nojdk-${version}-linux-arm64";
hash = "sha256-736PrTFckHyChRh0Uv8zNtCppQYhfZWECl9+44cs6Qo=";
hash = "sha256-6DzTEx218/Qq38eMWvXOX/t9VJDyPczz6Edh4eHdOfg=";
}
else if stdenv.hostPlatform.system == "x86_64-darwin" then
fetchurl {
url = "https://github.com/bazelbuild/bazel/releases/download/${version}/bazel-${version}-darwin-x86_64";
hash = "sha256-FX7ZKKG7uoteEvx0fBqpsoB3Gj0aJNaC2IXgJ2ffgz4=";
hash = "sha256-Ut00wXzJezqlvf49RcTjk4Im8j3Qv7R77t1iWpU/HwU=";
}
else
fetchurl {
# stdenv.hostPlatform.system == "aarch64-darwin"
url = "https://github.com/bazelbuild/bazel/releases/download/${version}/bazel-${version}-darwin-arm64";
hash = "sha256-+EP+HssT4aISUZwLKkSuuXjGQm9lheNJDr7WZw1v0pU=";
hash = "sha256-ArEXuX0JIa5NT04R0n4sCTA4HfQW43NDXV0EGcaibyQ=";
};

nativeBuildInputs = defaultShellUtils;
Expand Down Expand Up @@ -392,7 +393,7 @@ stdenv.mkDerivation rec {
})
]
# See enableNixHacks argument above.
++ lib.optional enableNixHacks ./nix-hacks.patch;
++ lib.optional enableNixHacks ./nix-build-bazel-package-hacks.patch;

postPatch =
let
Expand Down
Original file line number Diff line number Diff line change
@@ -1,46 +1,37 @@
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
index 4d8c46f8d5..ed311226f0 100644
index 53e6494656..22261cd268 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
@@ -178,18 +178,8 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
}
@@ -55,6 +55,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.io.IOException;
+import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.TreeMap;
@@ -193,16 +194,11 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
}

if (shouldUseCachedRepos(env, handler, repoRoot, rule)) {
- // Make sure marker file is up-to-date; correctly describes the current repository state
- byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env);
- if (env.valuesMissing()) {
- return null;
- }
- if (markerHash != null) { // repo exist & up-to-date
- return RepositoryDirectoryValue.builder()
- .setPath(repoRoot)
+ {
+ // Nix hack: Always consider cached dirs as up-to-date
return RepositoryDirectoryValue.builder()
.setPath(repoRoot)
- .setDigest(markerHash)
- .setExcludeFromVendoring(shouldExcludeRepoFromVendoring(handler, rule))
- .build();
- }
+ // Nix hack: Always consider cached dirs as up-to-date
+ return RepositoryDirectoryValue.builder().setPath(repoRoot).setDigest(digestWriter.writeMarkerFile()).build();
}

/* At this point: This is a force fetch, a local repository, OR The repository cache is old or
@@ -610,11 +600,12 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
builder.append(escape(key)).append(" ").append(escape(value)).append("\n");
- .setExcludeFromVendoring(excludeRepoFromVendoring)
+ .setDigest(digestWriter.writeMarkerFile(Collections.emptyMap()))
.build();
}
Comment on lines +23 to +31
Copy link
Contributor

@musjj musjj Nov 17, 2024

Choose a reason for hiding this comment

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

Have you tested if the patch works? It seems to generate an extra scope:

if (shouldUseCachedRepos(env, handler, repoRoot, rule)) {
  {
    // Nix hack: Always consider cached dirs as up-to-date
    return RepositoryDirectoryValue.builder()
        .setPath(repoRoot)
        .setDigest(digestWriter.writeMarkerFile(Collections.emptyMap()))
        .build();
  }
}

Does this actually return from the function or just from the scope (not really familiar with Java)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra nested block creates a nested lexical scope but return still refers to the enclosing function as there's no block-level returns. Added it to reduce formatting indentation diff against bazel upstream

}
String content = builder.toString();
- try {
- FileSystemUtils.writeContent(markerPath, UTF_8, content);
- } catch (IOException e) {
- throw new RepositoryFunctionException(e, Transience.TRANSIENT);
- }
+ // Nix hack: Do not write these pesky marker files
+ //try {
+ // FileSystemUtils.writeContent(markerPath, UTF_8, content);
+ //} catch (IOException e) {
+ // throw new RepositoryFunctionException(e, Transience.TRANSIENT);
+ //}
return new Fingerprint().addString(content).digestAndReset();
}



diff --git a/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
index 649647c5f2..64d05b530c 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/JavaSubprocessFactory.java
Expand Down