Skip to content

Commit d929084

Browse files
WyveraldWyverald
committed
misc fixes
1 parent 4437572 commit d929084

File tree

10 files changed

+31
-17
lines changed

10 files changed

+31
-17
lines changed

‎src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,7 @@ private String getAbsolutePath(String path, CommandEnvironment env) {
666666
*/
667667
@Nullable
668668
private Path toPath(PathFragment path, CommandEnvironment env) {
669-
if (path.isEmpty()) {
669+
if (path.isEmpty() || env.getBlazeWorkspace().getWorkspace() == null) {
670670
return null;
671671
}
672672
return env.getBlazeWorkspace().getWorkspace().getRelative(path);

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/VendorManager.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.google.devtools.build.lib.vfs.FileSystemUtils;
2929
import com.google.devtools.build.lib.vfs.Path;
3030
import com.google.devtools.build.lib.vfs.PathFragment;
31+
import com.google.devtools.build.lib.vfs.Symlinks;
3132
import java.io.IOException;
3233
import java.io.UnsupportedEncodingException;
3334
import java.net.URL;
@@ -86,8 +87,9 @@ public void vendorRepos(Path externalRepoRoot, ImmutableList<RepositoryName> rep
8687
if (isCached) {
8788
Path cacheRepoDir = repoUnderExternal.resolveSymbolicLinks();
8889
actualMarkerFile =
89-
cacheRepoDir.replaceName(
90-
cacheRepoDir.getBaseName() + RepoContentsCache.RECORDED_INPUTS_SUFFIX);
90+
cacheRepoDir
91+
.getParentDirectory()
92+
.getChild(cacheRepoDir.getBaseName() + RepoContentsCache.RECORDED_INPUTS_SUFFIX);
9193
} else {
9294
actualMarkerFile = markerUnderExternal;
9395
}
@@ -113,7 +115,8 @@ public void vendorRepos(Path externalRepoRoot, ImmutableList<RepositoryName> rep
113115
// 3. Move/copy the external repo to vendor dir. Note that, in the "move" case, it's fine if
114116
// this step fails or is interrupted, because the marker file under external is gone anyway.
115117
if (isCached) {
116-
FileSystemUtils.copyTreesBelow(repoUnderExternal.resolveSymbolicLinks(), repoUnderVendor);
118+
FileSystemUtils.copyTreesBelow(
119+
repoUnderExternal.resolveSymbolicLinks(), repoUnderVendor, Symlinks.NOFOLLOW);
117120
} else {
118121
try {
119122
repoUnderExternal.renameTo(repoUnderVendor);

‎src/main/java/com/google/devtools/build/lib/bazel/repository/cache/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ java_library(
2525
"//src/main/java/com/google/devtools/build/lib/util:file_system_lock",
2626
"//src/main/java/com/google/devtools/build/lib/vfs",
2727
"//src/main/java/com/google/devtools/build/lib/vfs/bazel",
28+
"//third_party:flogger",
2829
"//third_party:guava",
2930
"//third_party:jsr305",
3031
],

‎src/main/java/com/google/devtools/build/lib/bazel/repository/cache/RepoContentsCache.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
import com.google.common.base.Preconditions;
2020
import com.google.common.collect.ImmutableList;
21+
import com.google.common.flogger.GoogleLogger;
2122
import com.google.devtools.build.lib.server.IdleTask;
22-
import com.google.devtools.build.lib.server.IdleTaskException;
2323
import com.google.devtools.build.lib.util.FileSystemLock;
2424
import com.google.devtools.build.lib.util.FileSystemLock.LockMode;
2525
import com.google.devtools.build.lib.vfs.Dirent;
@@ -54,6 +54,7 @@
5454
* file is older than {@code --repo_contents_cache_gc_max_age} are garbage collected.
5555
*/
5656
public final class RepoContentsCache {
57+
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
5758
public static final String RECORDED_INPUTS_SUFFIX = ".recorded_inputs";
5859

5960
/**
@@ -99,7 +100,8 @@ private static CandidateRepo fromRecordedInputsFile(Path recordedInputsFile) {
99100
recordedInputsFileBaseName.substring(
100101
0, recordedInputsFileBaseName.length() - RECORDED_INPUTS_SUFFIX.length());
101102
return new CandidateRepo(
102-
recordedInputsFile, recordedInputsFile.replaceName(contentsDirBaseName));
103+
recordedInputsFile,
104+
recordedInputsFile.getParentDirectory().getChild(contentsDirBaseName));
103105
}
104106

105107
/** Updates the mtime of the recorded inputs file, to delay GC for this entry. */
@@ -212,18 +214,13 @@ public void releaseSharedLock() throws IOException {
212214
public IdleTask createGcIdleTask(Duration maxAge, Duration idleDelay) {
213215
Preconditions.checkState(path != null);
214216
return new IdleTask() {
215-
@Override
216-
public String displayName() {
217-
return "Repo contents cache garbage collection";
218-
}
219-
220217
@Override
221218
public Duration delay() {
222219
return idleDelay;
223220
}
224221

225222
@Override
226-
public void run() throws InterruptedException, IdleTaskException {
223+
public void run() {
227224
try {
228225
Preconditions.checkState(path != null);
229226
// If we can't grab the lock, abort GC. Someone will come along later.
@@ -234,8 +231,8 @@ public void run() throws InterruptedException, IdleTaskException {
234231
// be safe. At worst, multiple servers performing GC will try to delete the same files,
235232
// but whatever.
236233
path.getChild(TRASH_PATH).deleteTreesBelow();
237-
} catch (IOException e) {
238-
throw new IdleTaskException(e);
234+
} catch (IOException | InterruptedException e) {
235+
logger.atInfo().withCause(e).log("repo contents cache GC failed");
239236
}
240237
}
241238
};

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
3333
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
3434
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
35+
import com.google.devtools.build.lib.bazel.repository.cache.RepoContentsCache;
3536
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction;
3637
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule;
3738
import com.google.devtools.build.lib.clock.BlazeClock;
@@ -163,7 +164,8 @@ public void setup() throws Exception {
163164
new AtomicBoolean(true),
164165
ImmutableMap::of,
165166
directories,
166-
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER))
167+
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER,
168+
new RepoContentsCache()))
167169
.put(
168170
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
169171
new BzlmodRepoRuleFunction(ruleClassProvider, directories))

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode;
3333
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
3434
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
35+
import com.google.devtools.build.lib.bazel.repository.cache.RepoContentsCache;
3536
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction;
3637
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule;
3738
import com.google.devtools.build.lib.clock.BlazeClock;
@@ -234,7 +235,8 @@ public void setup() throws Exception {
234235
new AtomicBoolean(true),
235236
ImmutableMap::of,
236237
directories,
237-
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER))
238+
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER,
239+
new RepoContentsCache()))
238240
.put(
239241
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
240242
new BzlmodRepoRuleFunction(ruleClassProvider, directories))

‎src/test/java/com/google/devtools/build/lib/rules/repository/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ java_library(
3232
"//src/main/java/com/google/devtools/build/lib/events",
3333
"//src/main/java/com/google/devtools/build/lib/packages",
3434
"//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
35+
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
3536
"//src/main/java/com/google/devtools/build/lib/pkgcache",
3637
"//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",
3738
"//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",
@@ -49,6 +50,7 @@ java_library(
4950
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function",
5051
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
5152
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
53+
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
5254
"//src/main/java/com/google/devtools/build/lib/util/io",
5355
"//src/main/java/com/google/devtools/build/lib/vfs",
5456
"//src/main/java/com/google/devtools/build/skyframe",

‎src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,6 @@ private void loadRepo(String strippedRepoName) throws InterruptedException {
536536
evaluator.evaluate(ImmutableList.of(key), evaluationContext);
537537
assertThat(result.hasError()).isFalse();
538538
RepositoryDirectoryValue repositoryDirectoryValue = (RepositoryDirectoryValue) result.get(key);
539-
assertThat(repositoryDirectoryValue.repositoryExists()).isTrue();
539+
assertThat(repositoryDirectoryValue).isInstanceOf(Success.class);
540540
}
541541
}

‎src/test/java/com/google/devtools/build/lib/skyframe/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,7 @@ java_test(
11541154
"//src/main/java/com/google/devtools/build/lib/events",
11551155
"//src/main/java/com/google/devtools/build/lib/io:file_symlink_cycle_uniqueness_function",
11561156
"//src/main/java/com/google/devtools/build/lib/packages",
1157+
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
11571158
"//src/main/java/com/google/devtools/build/lib/pkgcache",
11581159
"//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",
11591160
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
@@ -1424,6 +1425,7 @@ java_test(
14241425
"//src/main/java/com/google/devtools/build/lib/events",
14251426
"//src/main/java/com/google/devtools/build/lib/io:file_symlink_cycle_uniqueness_function",
14261427
"//src/main/java/com/google/devtools/build/lib/packages",
1428+
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
14271429
"//src/main/java/com/google/devtools/build/lib/pkgcache",
14281430
"//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",
14291431
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
@@ -1447,6 +1449,7 @@ java_test(
14471449
"//src/test/java/com/google/devtools/build/lib:test_runner", # unuseddeps: keep buildcleaner: keep
14481450
"//src/test/java/com/google/devtools/build/lib/analysis/util",
14491451
"//src/test/java/com/google/devtools/build/lib/testutil",
1452+
"//src/test/java/com/google/devtools/build/skyframe:testutil",
14501453
"//third_party:guava",
14511454
"//third_party:guava-testlib",
14521455
"//third_party:junit4",

‎src/test/shell/bazel/workspace_resolved_test.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ mock_rules_java_to_avoid_downloading
2323

2424
disable_bzlmod
2525

26+
# disable the repo contents cache as it causes tests to hang
27+
# don't use them together, kids
28+
add_to_bazelrc 'common --repo_contents_cache='
29+
2630
test_result_recorded() {
2731
mkdir result_recorded && cd result_recorded
2832
rm -rf fetchrepo

0 commit comments

Comments
 (0)