Skip to content

Commit

Permalink
Support (workspace) relative paths in --override_module
Browse files Browse the repository at this point in the history
closes bazelbuild#17551

PiperOrigin-RevId: 519710834
Change-Id: I992d68e40b600ba921000d34d878186701baad2c
  • Loading branch information
SalmaSamy authored and copybara-github committed Mar 27, 2023
1 parent 16c639c commit f7627e0
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
// We use a LinkedHashMap to preserve the iteration order.
Map<RepositoryName, PathFragment> overrideMap = new LinkedHashMap<>();
for (RepositoryOverride override : repoOptions.repositoryOverrides) {
overrideMap.put(override.repositoryName(), override.path());
String repoPath = getAbsolutePath(override.path(), env);
overrideMap.put(override.repositoryName(), PathFragment.create(repoPath));
}
ImmutableMap<RepositoryName, PathFragment> newOverrides = ImmutableMap.copyOf(overrideMap);
if (!Maps.difference(overrides, newOverrides).areEqual()) {
Expand All @@ -406,9 +407,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {

if (repoOptions.moduleOverrides != null) {
Map<String, ModuleOverride> moduleOverrideMap = new LinkedHashMap<>();
for (RepositoryOptions.ModuleOverride modOverride : repoOptions.moduleOverrides) {
moduleOverrideMap.put(
modOverride.moduleName(), LocalPathOverride.create(modOverride.path()));
for (RepositoryOptions.ModuleOverride override : repoOptions.moduleOverrides) {
String modulePath = getAbsolutePath(override.path(), env);
moduleOverrideMap.put(override.moduleName(), LocalPathOverride.create(modulePath));
}
ImmutableMap<String, ModuleOverride> newModOverrides =
ImmutableMap.copyOf(moduleOverrideMap);
Expand Down Expand Up @@ -471,6 +472,21 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
}
}

/**
* If the given path is absolute path, leave it as it is. If the given path is a relative path, it
* is relative to the current working directory. If the given path starts with '%workspace%, it is
* relative to the workspace root, which is the output of `bazel info workspace`.
*
* @return Absolute Path
*/
private String getAbsolutePath(String path, CommandEnvironment env) {
path = path.replace("%workspace%", env.getWorkspace().getPathString());
if (!PathFragment.isAbsolute(path)) {
path = env.getWorkingDirectory().getRelative(path).getPathString();
}
return path;
}

@Override
public ImmutableList<Injected> getPrecomputedValues() {
return ImmutableList.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ public class RepositoryOptions extends OptionsBase {
converter = RepositoryOverrideConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Overrides a repository with a local directory.")
help =
"Override a repository with a local path in the form of <repository name>=<path>. If the"
+ " given path is an absolute path, it will be used as it is. If the given path is a"
+ " relative path, it is relative to the current working directory. If the given path"
+ " starts with '%workspace%, it is relative to the workspace root, which is the"
+ " output of `bazel info workspace`")
public List<RepositoryOverride> repositoryOverrides;

@Option(
Expand All @@ -161,7 +166,12 @@ public class RepositoryOptions extends OptionsBase {
converter = ModuleOverrideConverter.class,
documentationCategory = OptionDocumentationCategory.BZLMOD,
effectTags = {OptionEffectTag.UNKNOWN},
help = "Overrides a module with a local directory.")
help =
"Override a module with a local path in the form of <module name>=<path>. If the given"
+ " path is an absolute path, it will be used as it is. If the given path is a"
+ " relative path, it is relative to the current working directory. If the given path"
+ " starts with '%workspace%, it is relative to the workspace root, which is the"
+ " output of `bazel info workspace`")
public List<ModuleOverride> moduleOverrides;

@Option(
Expand Down Expand Up @@ -325,17 +335,10 @@ public RepositoryOverride convert(String input) throws OptionsParsingException {
throw new OptionsParsingException(
"Repository overrides must be of the form 'repository-name=path'", input);
}
OptionsUtils.AbsolutePathFragmentConverter absolutePathFragmentConverter =
new OptionsUtils.AbsolutePathFragmentConverter();
PathFragment path;
try {
path = absolutePathFragmentConverter.convert(pieces[1]);
} catch (OptionsParsingException e) {
throw new OptionsParsingException(
"Repository override directory must be an absolute path", input, e);
}
OptionsUtils.PathFragmentConverter pathConverter = new OptionsUtils.PathFragmentConverter();
String pathString = pathConverter.convert(pieces[1]).getPathString();
try {
return RepositoryOverride.create(RepositoryName.create(pieces[0]), path);
return RepositoryOverride.create(RepositoryName.create(pieces[0]), pathString);
} catch (LabelSyntaxException e) {
throw new OptionsParsingException("Invalid repository name given to override", input, e);
}
Expand Down Expand Up @@ -367,15 +370,9 @@ public ModuleOverride convert(String input) throws OptionsParsingException {
pieces[0]));
}

OptionsUtils.AbsolutePathFragmentConverter absolutePathFragmentConverter =
new OptionsUtils.AbsolutePathFragmentConverter();
try {
var path = absolutePathFragmentConverter.convert(pieces[1]);
return ModuleOverride.create(pieces[0], path.toString());
} catch (OptionsParsingException e) {
throw new OptionsParsingException(
"Module override directory must be an absolute path", input, e);
}
OptionsUtils.PathFragmentConverter pathConverter = new OptionsUtils.PathFragmentConverter();
String pathString = pathConverter.convert(pieces[1]).getPathString();
return ModuleOverride.create(pieces[0], pathString);
}

@Override
Expand All @@ -388,13 +385,13 @@ public String getTypeDescription() {
@AutoValue
public abstract static class RepositoryOverride {

private static RepositoryOverride create(RepositoryName repositoryName, PathFragment path) {
private static RepositoryOverride create(RepositoryName repositoryName, String path) {
return new AutoValue_RepositoryOptions_RepositoryOverride(repositoryName, path);
}

public abstract RepositoryName repositoryName();

public abstract PathFragment path();
public abstract String path();
}

/** A module override, represented by a name and an absolute path to a module. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,22 @@ public class RepositoryOptionsTest {
public void testOverrideConverter() throws Exception {
RepositoryOverride actual = converter.convert("foo=/bar");
assertThat(actual.repositoryName()).isEqualTo(RepositoryName.createUnvalidated("foo"));
assertThat(actual.path()).isEqualTo(PathFragment.create("/bar"));
assertThat(PathFragment.create(actual.path())).isEqualTo(PathFragment.create("/bar"));
}

@Test
public void testOverridePathWithEqualsSign() throws Exception {
RepositoryOverride actual = converter.convert("foo=/bar=/baz");
assertThat(actual.repositoryName()).isEqualTo(RepositoryName.createUnvalidated("foo"));
assertThat(actual.path()).isEqualTo(PathFragment.create("/bar=/baz"));
assertThat(PathFragment.create(actual.path())).isEqualTo(PathFragment.create("/bar=/baz"));
}

@Test
public void testOverridePathWithTilde() throws Exception {
RepositoryOverride actual = converter.convert("foo=~/bar");
assertThat(actual.repositoryName()).isEqualTo(RepositoryName.createUnvalidated("foo"));
assertThat(actual.path()).isEqualTo(PathFragment.create(USER_HOME.value() + "/bar"));
assertThat(PathFragment.create(actual.path()))
.isEqualTo(PathFragment.create(USER_HOME.value() + "/bar"));
}

@Test
Expand All @@ -70,6 +71,15 @@ public void testModuleOverridePathWithTilde() throws Exception {
.isEqualTo(PathFragment.create(USER_HOME.value() + "/bar"));
}

@Test
public void testModuleOverrideRelativePath() throws Exception {
var converter = new ModuleOverrideConverter();
ModuleOverride actual = converter.convert("foo=%workspace%/bar");
assertThat(actual.path()).isEqualTo("%workspace%/bar");
actual = converter.convert("foo=../../bar");
assertThat(actual.path()).isEqualTo("../../bar");
}

@Test
public void testInvalidOverride() throws Exception {
expectedException.expect(OptionsParsingException.class);
Expand All @@ -85,10 +95,4 @@ public void testInvalidRepoOverride() throws Exception {
converter.convert("foo/bar=/baz");
}

@Test
public void testInvalidPathOverride() throws Exception {
expectedException.expect(OptionsParsingException.class);
expectedException.expectMessage("Repository override directory must be an absolute path");
converter.convert("foo=bar");
}
}
91 changes: 85 additions & 6 deletions src/test/py/bazel/bzlmod/bazel_module_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@ def testRepositoryRuleErrorInModuleExtensionFailsTheBuild(self):
in line for line in stderr
]))

def testCommandLineModuleOverride(self):
def testCmdAbsoluteModuleOverride(self):
# test commandline_overrides takes precedence over local_path_override
self.ScratchFile('MODULE.bazel', [
'bazel_dep(name = "ss", version = "1.0")',
'local_path_override(',
Expand All @@ -416,15 +417,93 @@ def testCommandLineModuleOverride(self):
])
self.ScratchFile('bb/WORKSPACE')

_, _, stderr = self.RunBazel([
'build', '--experimental_enable_bzlmod', '@ss//:all',
'--override_module', 'ss=' + self.Path('bb')
],
allow_failure=False)
_, _, stderr = self.RunBazel(
['build', '@ss//:all', '--override_module', 'ss=' + self.Path('bb')],
allow_failure=False,
)
# module file override should be ignored, and bb directory should be used
self.assertIn(
'Target @ss~override//:choose_me up-to-date (nothing to build)', stderr)

def testCmdRelativeModuleOverride(self):
self.ScratchFile('aa/WORKSPACE')
self.ScratchFile(
'aa/MODULE.bazel',
[
'bazel_dep(name = "ss", version = "1.0")',
],
)
self.ScratchFile('aa/BUILD')

self.ScratchFile('aa/cc/BUILD')

self.ScratchFile('bb/WORKSPACE')
self.ScratchFile(
'bb/MODULE.bazel',
[
'module(name="ss")',
],
)
self.ScratchFile(
'bb/BUILD',
[
'filegroup(name = "choose_me")',
],
)

_, _, stderr = self.RunBazel(
[
'build',
'@ss//:all',
'--override_module',
'ss=../../bb',
'--enable_bzlmod',
],
cwd=self.Path('aa/cc'),
allow_failure=False,
)
self.assertIn(
'Target @ss~override//:choose_me up-to-date (nothing to build)', stderr
)

def testCmdWorkspaceRelativeModuleOverride(self):
self.ScratchFile('WORKSPACE')
self.ScratchFile(
'MODULE.bazel',
[
'bazel_dep(name = "ss", version = "1.0")',
],
)
self.ScratchFile('BUILD')
self.ScratchFile('aa/BUILD')
self.ScratchFile('bb/WORKSPACE')
self.ScratchFile(
'bb/MODULE.bazel',
[
'module(name="ss")',
],
)
self.ScratchFile(
'bb/BUILD',
[
'filegroup(name = "choose_me")',
],
)

_, _, stderr = self.RunBazel(
[
'build',
'@ss//:all',
'--override_module',
'ss=%workspace%/bb',
],
cwd=self.Path('aa'),
allow_failure=False,
)
self.assertIn(
'Target @ss~override//:choose_me up-to-date (nothing to build)', stderr
)

def testDownload(self):
data_path = self.ScratchFile('data.txt', ['some data'])
data_url = pathlib.Path(data_path).resolve().as_uri()
Expand Down
18 changes: 11 additions & 7 deletions src/test/shell/bazel/workspace_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -263,29 +263,33 @@ local_repository(
path = "original",
)
EOF
# Test absolute path
bazel build --override_repository="o=$PWD/override" @o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "override" bazel-genfiles/external/o/gen.out

# Test no override used
bazel build @o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "original" bazel-genfiles/external/o/gen.out

bazel build --override_repository="o=$PWD/override" @o//:gen &> $TEST_log \
# Test relative path (should be relative to working directory)
bazel build --override_repository="o=override" @o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "override" bazel-genfiles/external/o/gen.out

bazel build @o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "original" bazel-genfiles/external/o/gen.out

# For multiple override options, the latest should win
bazel build --override_repository=o=/ignoreme \
--override_repository="o=$PWD/override" \
@o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "override" bazel-genfiles/external/o/gen.out

# Test workspace relative path
mkdir -p dummy
cd dummy
bazel build --override_repository="o=%workspace%/override" @o//:gen &> $TEST_log \
|| fail "Expected build to succeed"
assert_contains "override" ../bazel-genfiles/external/o/gen.out

}

function test_workspace_override_starlark(){
Expand Down

0 comments on commit f7627e0

Please sign in to comment.