Skip to content

Commit

Permalink
[6.4.0] Cherry pick Bzlmod fixes (#19494)
Browse files Browse the repository at this point in the history
Cherry-picked commits:

- 15b1575 
- acb38a6
- 07e0d31

---------

Co-authored-by: salma-samy <salmasamy@google.com>
  • Loading branch information
meteorcloudy and SalmaSamy authored Sep 12, 2023
1 parent f465abd commit 0d70b76
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
throw new BazelDepGraphFunctionException(
ExternalDepsException.withMessage(
Code.BAD_MODULE,
"Lock file is no longer up-to-date because: %s",
"Lock file is no longer up-to-date because: %s. "
+ "Please run `bazel mod deps --lockfile_mode=update` to update your lockfile.",
String.join(", ", diffLockfile)),
Transience.PERSISTENT);
}
Expand Down Expand Up @@ -129,6 +130,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (!lockfileMode.equals(LockfileMode.OFF)) {
BazelLockFileValue updateLockfile =
lockfile.toBuilder()
.setLockFileVersion(BazelLockFileValue.LOCK_FILE_VERSION)
.setModuleFileHash(root.getModuleFileHash())
.setFlags(flags)
.setLocalOverrideHashes(localOverrideHashes)
Expand Down Expand Up @@ -183,7 +185,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
ImmutableMap<String, String> moduleOverrides =
ModuleFileFunction.MODULE_OVERRIDES.get(env).entrySet().stream()
.collect(
toImmutableMap(e -> e.getKey(), e -> ((LocalPathOverride) e.getValue()).getPath()));
toImmutableMap(Entry::getKey, e -> ((LocalPathOverride) e.getValue()).getPath()));

ImmutableList<String> yankedVersions =
ImmutableList.copyOf(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS.get(env));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,18 @@
import com.google.gson.JsonSyntaxException;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;

/** Reads the contents of the lock file into its value. */
public class BazelLockFileFunction implements SkyFunction {

public static final Precomputed<LockfileMode> LOCKFILE_MODE = new Precomputed<>("lockfile_mode");

private static final Pattern LOCKFILE_VERSION_PATTERN =
Pattern.compile("\"lockFileVersion\":\\s*(\\d+)");

private final Path rootDirectory;

private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS =
Expand Down Expand Up @@ -95,13 +100,21 @@ public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath) throw
BazelLockFileValue bazelLockFileValue;
try {
String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8);
bazelLockFileValue =
GsonTypeAdapterUtil.createLockFileGson(
lockfilePath
.asPath()
.getParentDirectory()
.getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME))
.fromJson(json, BazelLockFileValue.class);
Matcher matcher = LOCKFILE_VERSION_PATTERN.matcher(json);
int version = matcher.find() ? Integer.parseInt(matcher.group(1)) : -1;
if (version == BazelLockFileValue.LOCK_FILE_VERSION) {
bazelLockFileValue =
GsonTypeAdapterUtil.createLockFileGson(
lockfilePath
.asPath()
.getParentDirectory()
.getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME))
.fromJson(json, BazelLockFileValue.class);
} else {
// This is an old version, needs to be updated
// Keep old version to recognize the problem in error mode
bazelLockFileValue = EMPTY_LOCKFILE.toBuilder().setLockFileVersion(version).build();
}
} catch (FileNotFoundException e) {
bazelLockFileValue = EMPTY_LOCKFILE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ public ImmutableList<String> getModuleAndFlagsDiff(
ImmutableMap<String, String> localOverrideHashes,
BzlmodFlagsAndEnvVars flags) {
ImmutableList.Builder<String> moduleDiff = new ImmutableList.Builder<>();
if (getLockFileVersion() != BazelLockFileValue.LOCK_FILE_VERSION) {
return moduleDiff
.add("the version of the lockfile is not compatible with the current Bazel")
.build();
}
if (!moduleFileHash.equals(getModuleFileHash())) {
moduleDiff.add("the root MODULE.bazel has been modified");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile(
throw new SingleExtensionEvalFunctionException(
ExternalDepsException.withMessage(
Code.BAD_MODULE,
"Lock file is no longer up-to-date because: %s",
"Lock file is no longer up-to-date because: %s. "
+ "Please run `bazel mod deps --lockfile_mode=update` to update your lockfile.",
String.join(", ", extDiff)),
Transience.PERSISTENT);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private static ImmutableList<String> readFile(Path file) throws IOException {
}

private static void writeFile(Path file, List<String> content) throws IOException {
FileSystemUtils.writeLinesAs(file, UTF_8, content);
FileSystemUtils.writeLinesAs(file, UTF_8, content, "\n");
}

private static boolean getReadPermission(int permission) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -708,17 +708,17 @@ public static void writeContent(Path outputFile, byte[] content) throws IOExcept
}

/**
* Writes lines to file using the given encoding, ending every line with a line break '\n'
* character.
* Writes lines to file using the given encoding, ending every line with a system specific line
* break character.
*/
@ThreadSafe // but not atomic
public static void writeLinesAs(Path file, Charset charset, String... lines) throws IOException {
writeLinesAs(file, charset, Arrays.asList(lines));
}

/**
* Writes lines to file using the given encoding, ending every line with a line break '\n'
* character.
* Writes lines to file using the given encoding, ending every line with a system specific line
* break character.
*/
@ThreadSafe // but not atomic
public static void writeLinesAs(Path file, Charset charset, Iterable<String> lines)
Expand All @@ -728,17 +728,28 @@ public static void writeLinesAs(Path file, Charset charset, Iterable<String> lin
}

/**
* Appends lines to file using the given encoding, ending every line with a line break '\n'
* Writes lines to file using the given encoding, ending every line with a given line break
* character.
*/
@ThreadSafe // but not atomic
public static void writeLinesAs(
Path file, Charset charset, Iterable<String> lines, String lineBreak) throws IOException {
file.getParentDirectory().createDirectoryAndParents();
asByteSink(file).asCharSink(charset).writeLines(lines, lineBreak);
}

/**
* Appends lines to file using the given encoding, ending every line with a system specific line
* break character.
*/
@ThreadSafe // but not atomic
public static void appendLinesAs(Path file, Charset charset, String... lines) throws IOException {
appendLinesAs(file, charset, Arrays.asList(lines));
}

/**
* Appends lines to file using the given encoding, ending every line with a line break '\n'
* character.
* Appends lines to file using the given encoding, ending every line with a system specific line
* break character.
*/
@ThreadSafe // but not atomic
public static void appendLinesAs(Path file, Charset charset, Iterable<String> lines)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,9 @@ public void invalidLockfileEmptyFile() throws Exception {
fail(result.getError().toString());
}

scratch.overwriteFile(rootDirectory.getRelative("MODULE.bazel.lock").getPathString(), "{}");
scratch.overwriteFile(
rootDirectory.getRelative("MODULE.bazel.lock").getPathString(),
"{\"lockFileVersion\": " + BazelLockFileValue.LOCK_FILE_VERSION + "}");

result = evaluator.evaluate(ImmutableList.of(BazelLockFileValue.KEY), evaluationContext);
if (!result.hasError()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ public void testPatch() throws Exception {
context.createFile(
context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread);
context.patch(patchFile, StarlarkInt.of(0), thread);
testOutputFile(foo.getPath(), String.format("line one%nline two%n"));
testOutputFile(foo.getPath(), "line one\nline two\n");
}

@Test
Expand Down
46 changes: 42 additions & 4 deletions src/test/py/bazel/bzlmod/bazel_lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ def testLockfileErrorMode(self):
'ERROR: Error computing the main repository mapping: Lock file is'
' no longer up-to-date because: the root MODULE.bazel has been'
' modified, the value of --check_direct_dependencies flag has'
' been modified'
' been modified. Please run'
' `bazel mod deps --lockfile_mode=update` to update your lockfile.'
),
stderr,
)
Expand Down Expand Up @@ -251,7 +252,8 @@ def testLocalOverrideWithErrorMode(self):
(
'ERROR: Error computing the main repository mapping: Lock file is'
' no longer up-to-date because: The MODULE.bazel file has changed'
' for the overriden module: bar'
' for the overriden module: bar. Please run'
' `bazel mod deps --lockfile_mode=update` to update your lockfile.'
),
stderr,
)
Expand Down Expand Up @@ -502,7 +504,8 @@ def testUpdateModuleExtensionErrorMode(self):
'implementation of the extension '
"'ModuleExtensionId{bzlFileLabel=//:extension.bzl, "
"extensionName=lockfile_ext, isolationKey=Optional.empty}' or one "
'of its transitive .bzl files has changed'
'of its transitive .bzl files has changed. Please run'
' `bazel mod deps --lockfile_mode=update` to update your lockfile.'
),
stderr,
)
Expand Down Expand Up @@ -688,7 +691,8 @@ def testChangeEnvVariableInErrorMode(self):
' variables the extension'
" 'ModuleExtensionId{bzlFileLabel=//:extension.bzl,"
" extensionName=lockfile_ext, isolationKey=Optional.empty}' depends"
' on (or their values) have changed'
' on (or their values) have changed. Please run'
' `bazel mod deps --lockfile_mode=update` to update your lockfile.'
),
stderr,
)
Expand Down Expand Up @@ -735,6 +739,40 @@ def testModuleExtensionWithFile(self):
_, _, stderr = self.RunBazel(['build', '@hello//:all'])
self.assertIn('I have changed now!', ''.join(stderr))

def testOldVersion(self):
self.ScratchFile('MODULE.bazel')
self.ScratchFile('BUILD', ['filegroup(name = "hello")'])
self.RunBazel(['build', '--nobuild', '//:all'])

# Set version to old
with open('MODULE.bazel.lock', 'r') as json_file:
data = json.load(json_file)
data['lockFileVersion'] = 0
with open('MODULE.bazel.lock', 'w') as json_file:
json.dump(data, json_file, indent=4)

# Run in error mode
exit_code, _, stderr = self.RunBazel(
['build', '--nobuild', '--lockfile_mode=error', '//:all'],
allow_failure=True,
)
self.AssertExitCode(exit_code, 48, stderr)
self.assertIn(
(
'ERROR: Error computing the main repository mapping: Lock file is'
' no longer up-to-date because: the version of the lockfile is not'
' compatible with the current Bazel. Please run'
' `bazel mod deps --lockfile_mode=update` to update your lockfile.'
),
stderr,
)

# Run again with update
self.RunBazel(['build', '--nobuild', '//:all'])
with open('MODULE.bazel.lock', 'r') as json_file:
data = json.load(json_file)
self.assertEqual(data['lockFileVersion'], 1)


def testExtensionEvaluationDoesNotRerunOnChangedImports(self):
"""
Expand Down

0 comments on commit 0d70b76

Please sign in to comment.