Skip to content

Commit

Permalink
Refactor fetch command specially fetchTarget function
Browse files Browse the repository at this point in the history
This would also allow re-using the fetch target logic for vendoring target

PiperOrigin-RevId: 607740129
Change-Id: Ib73ee4ad6fa4d015eee97bcfd41e1be897f7c770
  • Loading branch information
SalmaSamy authored and copybara-github committed Feb 16, 2024
1 parent f5c8deb commit 5cb37c1
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 194 deletions.
4 changes: 2 additions & 2 deletions site/en/run/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ With Bazel 7 or later, if you have Bzlmod enabled, you can also fetch all
external dependencies by running

```posix-terminal
bazel fetch --all
bazel fetch
```

You do not need to run bazel fetch at all if you have all of the tools you are
Expand Down Expand Up @@ -407,7 +407,7 @@ touch WORKSPACE
To fetch built-in Bzlmod dependencies, run

```posix-terminal
bazel fetch --all --repository_cache="path/to/repository/cache"
bazel fetch --repository_cache="path/to/repository/cache"
```

If you still rely on the legacy WORKSPACE file, to fetch built-in WORKSPACE
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import com.google.devtools.common.options.OptionsBase;
import java.util.List;

/** Defines the options specific to Bazel's sync command */
/** Defines the options specific to Bazel's fetch command */
public class FetchOptions extends OptionsBase {

@Option(
name = "all",
defaultValue = "false",
Expand All @@ -34,7 +35,7 @@ public class FetchOptions extends OptionsBase {
@Option(
name = "configure",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
documentationCategory = OptionDocumentationCategory.BZLMOD,
effectTags = {OptionEffectTag.CHANGES_INPUTS},
help =
"Only fetches repositories marked as 'configure' for system-configuration purpose. Only"
Expand All @@ -45,7 +46,7 @@ public class FetchOptions extends OptionsBase {
name = "repo",
defaultValue = "null",
allowMultiple = true,
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
documentationCategory = OptionDocumentationCategory.BZLMOD,
effectTags = {OptionEffectTag.CHANGES_INPUTS},
help =
"Only fetches the specified repository, which can be either {@apparent_repo_name} or"
Expand All @@ -55,7 +56,7 @@ public class FetchOptions extends OptionsBase {
@Option(
name = "force",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
documentationCategory = OptionDocumentationCategory.BZLMOD,
effectTags = {OptionEffectTag.CHANGES_INPUTS},
help =
"Ignore existing repository if any and force fetch the repository again. Only works when "
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
// Copyright 2024 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.devtools.build.lib.bazel.commands;

import com.google.common.base.Joiner;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.cmdline.TargetPattern;
import com.google.devtools.build.lib.cmdline.TargetPattern.Parser;
import com.google.devtools.build.lib.packages.LabelPrinter;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.query2.common.AbstractBlazeQueryEnvironment;
import com.google.devtools.build.lib.query2.common.UniverseScope;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.Setting;
import com.google.devtools.build.lib.query2.engine.QueryEvalResult;
import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.query2.engine.QueryExpression;
import com.google.devtools.build.lib.query2.engine.QuerySyntaxException;
import com.google.devtools.build.lib.query2.engine.ThreadSafeOutputFormatterCallback;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.KeepGoingOption;
import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
import com.google.devtools.build.lib.runtime.commands.QueryCommand;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue.RepositoryMappingResolutionException;
import com.google.devtools.common.options.OptionsParsingResult;
import java.io.IOException;
import java.util.EnumSet;
import java.util.List;

/** Fetches all repos needed for building a given set of targets. */
public class TargetFetcher {
private final CommandEnvironment env;

private TargetFetcher(CommandEnvironment env) {
this.env = env;
}

/** Uses `deps` query to find and fetch all repos needed for these targets */
public static void fetchTargets(
CommandEnvironment env, OptionsParsingResult options, List<String> targets)
throws RepositoryMappingResolutionException, InterruptedException, TargetFetcherException {
new TargetFetcher(env).fetchTargets(options, targets);
}

private void fetchTargets(OptionsParsingResult options, List<String> targets)
throws InterruptedException, TargetFetcherException, RepositoryMappingResolutionException {
AbstractBlazeQueryEnvironment<Target> queryEnv = getQueryEnv(options);
QueryExpression expr = createQueryExpression(targets, queryEnv);
QueryEvalResult queryEvalResult;
try {
queryEvalResult =
queryEnv.evaluateQuery(
expr,
new ThreadSafeOutputFormatterCallback<>() {
@Override
public void processOutput(Iterable<Target> partialResult) {}
});
} catch (IOException e) {
// Should be impossible since our OutputFormatterCallback doesn't throw IOException.
throw new IllegalStateException(e);
} catch (QueryException e) {
throw new TargetFetcherException(
String.format(
"Fetching target dependencies for %s encountered an error: %s",
expr, e.getMessage()));
}

if (!queryEvalResult.getSuccess()) {
throw new TargetFetcherException(
String.format(
"Fetching some target dependencies for %s failed, but --keep_going specified. "
+ " Ignoring errors",
expr));
}
}

AbstractBlazeQueryEnvironment<Target> getQueryEnv(OptionsParsingResult options)
throws RepositoryMappingResolutionException, InterruptedException {
boolean keepGoing = options.getOptions(KeepGoingOption.class).keepGoing;
LoadingPhaseThreadsOption threadsOption = options.getOptions(LoadingPhaseThreadsOption.class);
RepositoryMapping repoMapping =
env.getSkyframeExecutor()
.getMainRepoMapping(keepGoing, threadsOption.threads, env.getReporter());
TargetPattern.Parser targetParser =
new Parser(env.getRelativeWorkingDirectory(), RepositoryName.MAIN, repoMapping);
return QueryCommand.newQueryEnvironment(
env,
keepGoing,
false,
UniverseScope.EMPTY,
threadsOption.threads,
EnumSet.noneOf(Setting.class),
/* useGraphlessQuery= */ true,
targetParser,
LabelPrinter.legacy());
}

private QueryExpression createQueryExpression(
List<String> targets, AbstractBlazeQueryEnvironment<Target> queryEnv)
throws TargetFetcherException {
String query = "deps(" + Joiner.on(" union ").join(targets) + ")";
try {
return QueryExpression.parse(query, queryEnv);
} catch (QuerySyntaxException e) {
throw new TargetFetcherException(
String.format(
"Fetching target dependencies for %s encountered an error: %s",
QueryExpression.truncate(query), e.getMessage()));
}
}

static class TargetFetcherException extends Exception {
public TargetFetcherException(String message) {
super(message);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,6 @@ public SkyValue compute(SkyKey skyKey, Environment env)
.setDigest(markerHash)
.setExcludeFromVendoring(shouldExcludeRepoFromVendoring(handler, rule))
.build();
} else {
// So that we are in a consistent state if something happens while fetching the repository
DigestWriter.clearMarkerFile(directories, repositoryName);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/test/py/bazel/bzlmod/bazel_fetch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def testFetchAll(self):
],
)

self.RunBazel(['fetch', '--all'])
self.RunBazel(['fetch'])
_, stdout, _ = self.RunBazel(['info', 'output_base'])
repos_fetched = os.listdir(stdout[0] + '/external')
self.assertIn('aaa~', repos_fetched)
Expand Down Expand Up @@ -151,15 +151,15 @@ def testFetchFailsWithMultipleOptions(self):
)
self.AssertExitCode(exit_code, 2, stderr)
self.assertIn(
'ERROR: Only one fetch option should be provided for fetch command.',
'ERROR: Only one fetch option can be provided for fetch command',
stderr,
)
exit_code, _, stderr = self.RunBazel(
['fetch', '--all', '--repo=@hello'], allow_failure=True
['fetch', '//sometarget', '--repo=@hello'], allow_failure=True
)
self.AssertExitCode(exit_code, 2, stderr)
self.assertIn(
'ERROR: Only one fetch option should be provided for fetch command.',
'ERROR: Only one fetch option can be provided for fetch command',
stderr,
)

Expand Down Expand Up @@ -211,8 +211,8 @@ def testFetchInvalidRepo(self):
)
self.AssertExitCode(exit_code, 8, stderr)
self.assertIn(
"ERROR: Fetching repos failed with errors: Repository '@@nono' is not "
"defined; No repository visible as '@nana' from main repository",
"ERROR: Fetching some repos failed with errors: Repository '@@nono' is "
"not defined; No repository visible as '@nana' from main repository",
stderr,
)

Expand Down
10 changes: 5 additions & 5 deletions src/test/shell/bazel/bazel_repository_cache_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ function test_fetch() {

bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"
expect_log "All external dependencies fetched successfully"
expect_log "All external dependencies for these targets fetched successfully"
}

function test_directory_structure() {
Expand Down Expand Up @@ -231,7 +231,7 @@ function test_fetch_value_with_existing_cache_and_no_network() {
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

expect_log "All external dependencies fetched successfully"
expect_log "All external dependencies for these targets fetched successfully"
}


Expand All @@ -249,7 +249,7 @@ function test_load_cached_value() {
bazel fetch --repository_cache="$repo_cache_dir" //zoo:breeding-program >& $TEST_log \
|| echo "Expected fetch to succeed"

expect_log "All external dependencies fetched successfully"
expect_log "All external dependencies for these targets fetched successfully"
}

function test_write_cache_without_hash() {
Expand Down Expand Up @@ -388,7 +388,7 @@ EOF
bazel fetch --repository_cache="$repo_cache_dir" @foo//:all >& $TEST_log \
|| echo "Expected fetch to succeed"

expect_log "All external dependencies fetched successfully"
expect_log "All external dependencies for these targets fetched successfully"
}

function test_starlark_download_fail_without_cache() {
Expand Down Expand Up @@ -441,7 +441,7 @@ EOF
bazel fetch --repository_cache="$repo_cache_dir" @foo//:all >& $TEST_log \
|| echo "Expected fetch to succeed"

expect_log "All external dependencies fetched successfully"
expect_log "All external dependencies for these targets fetched successfully"
}

function test_starlark_download_and_extract_fail_without_cache() {
Expand Down

0 comments on commit 5cb37c1

Please sign in to comment.