Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude convenience symlinks after changing the output base #21005

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,14 @@ public ProcessPackageDirectoryResult getPackageExistenceAndSubdirDeps(

PackageIdentifier packageId = PackageIdentifier.create(repositoryName, rootRelativePath);

if (packageId.getRepository().isMain()
&& fileValue.isSymlink()
&& fileValue
.getUnresolvedLinkTarget()
.startsWith(directories.getExecRootBase().asFragment())) {
// Symlinks back to the execroot are not traversed so that we avoid convenience symlinks.
// Note that it's not enough to just check for the convenience symlinks themselves,
// because if the value of --symlink_prefix changes, the old symlinks are left in place. This
// algorithm also covers more creative use cases where people create convenience symlinks
// somewhere in the directory tree manually.
if (packageId.getRepository().isMain() && isConvenienceSymlink(fileValue, rootedPath, env)) {
return ProcessPackageDirectoryResult.EMPTY_RESULT;
}

if(env.valuesMissing()) {
return null;
}

SkyKey pkgLookupKey = PackageLookupValue.key(packageId);
SkyKey dirListingKey = DirectoryListingValue.key(rootedPath);
SkyframeLookupResult pkgLookupAndDirectoryListingDeps =
Expand Down Expand Up @@ -187,6 +182,74 @@ public ProcessPackageDirectoryResult getPackageExistenceAndSubdirDeps(
/*additionalValuesToAggregate=*/ ImmutableMap.of());
}

// Note that it's not enough to just check for the convenience symlinks themselves,
// because if the value of --symlink_prefix changes, the old symlinks are left in place. It
// is also not sufficient to check whether the symlink points to a directory in the current
// exec root, since this can change between bazel invocations. Therefore we check if the
// suffix of the symlink source suggests it is a convenience symlink, then see if the symlink
// target is in a directory that looks like an execroot. This algorithm also covers more
// creative use cases where people create convenience symlinks somewhere in the directory
// tree manually.
private boolean isConvenienceSymlink(FileValue fileValue, RootedPath rootedPath, SkyFunction.Environment env) throws InterruptedException {
if (!fileValue.isSymlink()) {
return false;
}

PathFragment linkTarget = fileValue.getUnresolvedLinkTarget();

if(linkTarget.startsWith(directories.getExecRootBase().asFragment())) {
return true;
}

PathFragment rootRelativePath = rootedPath.getRootRelativePath();
Root root = rootedPath.getRoot();

if(rootRelativePath.getBaseName().endsWith("-bin") && isInExecRoot(linkTarget, root, 4, env)) {
return true;
}

if(rootRelativePath.getBaseName().endsWith("-genfiles") && isInExecRoot(linkTarget, root, 4, env)) {
return true;
}

if(rootRelativePath.getBaseName().endsWith("-out") && isInExecRoot(linkTarget, root, 2, env)) {
return true;
}

if(rootRelativePath.getBaseName().endsWith("-testlogs") && isInExecRoot(linkTarget, root, 4, env)) {
return true;
}

if(rootRelativePath.getBaseName().endsWith("-" + directories.getWorkingDirectory().getBaseName()) && isInExecRoot(linkTarget, root, 1, env)) {
return true;
}

return false;
}

private boolean isInExecRoot(PathFragment path, Root root, int depth, SkyFunction.Environment env) throws InterruptedException {
int segmentCount = path.segmentCount();

if(segmentCount <= depth) {
return false;
}

PathFragment candidateExecRoot = path.subFragment(0, segmentCount - depth);

if(!candidateExecRoot.getBaseName().equals("execroot")) {
return false;
}

Root absoluteRoot = Root.absoluteRoot(root.getFileSystem());
RootedPath doNotBuildPath = RootedPath.toRootedPath(absoluteRoot, candidateExecRoot.getChild("DO_NOT_BUILD_HERE"));
cameron-martin marked this conversation as resolved.
Show resolved Hide resolved
FileValue doNotBuildValue = (FileValue)env.getValue(FileValue.key(doNotBuildPath));
if (doNotBuildValue == null) {
return false;
}

return doNotBuildValue.exists();
}

private Iterable<SkyKey> getSubdirDeps(
DirectoryListingValue dirListingValue,
RootedPath rootedPath,
Expand Down
14 changes: 14 additions & 0 deletions src/test/py/bazel/query_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import os
import tempfile
from absl.testing import absltest
from src.test.py.bazel import test_base

Expand Down Expand Up @@ -129,6 +131,18 @@ def testBuildFilesForExternalRepos_NoDuplicates(self):
self.assertNotIn(item, result)
result.add(item)

def testQueryWithDifferentOutputBaseAfterBuilding(self):
output_base = tempfile.mkdtemp(dir = os.getenv("TEST_TMPDIR"))

self.ScratchFile('MODULE.bazel')
self.ScratchFile('BUILD', [
'py_binary(name="a", srcs=["a.py"])',
])
self.ScratchFile('a.py')
self.RunBazel(["build", "//..."])
self.RunBazel([f"--output_base={output_base}", "query", "//..."])


def _AssertQueryOutput(self, query_expr, *expected_results):
_, stdout, _ = self.RunBazel(['query', query_expr])

Expand Down
Loading