diff options
author | Damien Martin-Guillerez <dmarting@google.com> | 2016-02-25 12:14:42 +0000 |
---|---|---|
committer | Philipp Wollermann <philwo@google.com> | 2016-02-25 14:16:13 +0000 |
commit | 26f7f4849d4f56d23d21111ee17248ab64065009 (patch) | |
tree | 210a152f3ff5773c63616ef2887414fa6dab145c /src | |
parent | ed51bd75a24b5fb8ccf4fd38bf6140697f8f7de0 (diff) |
Testing correct invalidation of Skylark Remote Repositories
A Skylark remote repository should be invalidated only when
the WORKSPACE file change, or one of its dependency or the Skylark
file change.
This change include two fixes:
- The path of the RepositoryDirectoryValue was incorrect when
the directory root is a symlink and the repository is not local
(and not refetching). This was never triggered before because
the only rule that were symlinking their root were the local
one.
- Directories were unitialized for the SkylarkRepositoryFunction
(was forgotten as part of a refactor when introducing it).
--
MOS_MIGRATED_REVID=115547540
Diffstat (limited to 'src')
3 files changed, 37 insertions, 8 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 5e16483f6a..6d7c4f8886 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -110,6 +110,7 @@ public class BazelRepositoryModule extends BlazeModule { for (RepositoryFunction handler : repositoryHandlers.values()) { handler.setDirectories(directories); } + skylarkRepositoryFunction.setDirectories(directories); } /** 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 c5cd01ef05..8c66b96469 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 @@ -123,15 +123,12 @@ public class RepositoryDelegatorFunction implements SkyFunction { if (markerUpToDate && repoRoot.exists()) { // Now that we know that it exists, we can declare a Skyframe dependency on the repository // root. - FileValue repoRootValue = RepositoryFunction.getRepositoryDirectory(repoRoot, env); + RepositoryFunction.getRepositoryDirectory(repoRoot, env); if (env.valuesMissing()) { return null; } - // NB: This returns the wrong repository value for non-local new_* repository functions. - // This should sort itself out automatically once the ExternalFilesHelper refactoring is - // finally submitted. - return RepositoryDirectoryValue.create(repoRootValue.realRootedPath().asPath()); + return RepositoryDirectoryValue.create(repoRoot); } if (isFetch.get()) { diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh index f99e0796df..64590ae856 100755 --- a/src/test/shell/bazel/skylark_repository_test.sh +++ b/src/test/shell/bazel/skylark_repository_test.sh @@ -298,18 +298,49 @@ def _impl(ctx): print(ctx.os.environ["FOO"]) # Symlink so a repository is created ctx.symlink(ctx.path("$repo2"), ctx.path("")) -repo = repository_rule(implementation=_impl, local=True) +repo = repository_rule(implementation=_impl, local=False) EOF - bazel shutdown + # TODO(dmarting): We should seriously have something better to force a refetch... + bazel clean --expunge FOO=BAR bazel build @foo//:bar >& $TEST_log || fail "Failed to build" expect_log "BAR" - FOO=BAR bazel clean >& $TEST_log + FOO=BAR bazel clean --expunge >& $TEST_log FOO=BAR bazel info >& $TEST_log FOO=BAZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" expect_log "BAZ" + + # Test that we don't re-run on server restart. + FOO=BEZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" + expect_not_log "BEZ" + bazel shutdown >& $TEST_log + FOO=BEZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" + expect_not_log "BEZ" + + # Test modifying test.bzl invalidate the repository + cat >test.bzl <<EOF +def _impl(ctx): + print(ctx.os.environ["BAR"]) + # Symlink so a repository is created + ctx.symlink(ctx.path("$repo2"), ctx.path("")) +repo = repository_rule(implementation=_impl, local=True) +EOF + BAR=BEZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" + expect_log "BEZ" + + # Shutdown and modify again + bazel shutdown + cat >test.bzl <<EOF +def _impl(ctx): + print(ctx.os.environ["BAZ"]) + # Symlink so a repository is created + ctx.symlink(ctx.path("$repo2"), ctx.path("")) +repo = repository_rule(implementation=_impl, local=True) +EOF + BAZ=BOZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" + expect_log "BOZ" } function tear_down() { |