Skip to content

Commit dd24a00

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Test and fix root symlink edge case in runfiles library
Due to incomplete test coverage for the case of a root symlink named like a Bazel module, the Bash runfiles libraries behaved incorrectly in this case. This commit fixes that and adds proper test coverage for all libraries by adding repo mappings that conflict with the existing rlocation testcase for `config.json`. Closes bazelbuild#17317. PiperOrigin-RevId: 505873412 Change-Id: I72019d329b72cab9cf893deb714da8889d371617
1 parent 8e41dce commit dd24a00

File tree

4 files changed

+54
-21
lines changed

4 files changed

+54
-21
lines changed

‎tools/bash/runfiles/runfiles.bash

+4-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ function rlocation() {
142142

143143
if [[ -f "$RUNFILES_REPO_MAPPING" ]]; then
144144
local -r target_repo_apparent_name=$(echo "$1" | cut -d / -f 1)
145-
local -r remainder=$(echo "$1" | cut -d / -f 2-)
145+
# Use -s to get an empty remainder if the argument does not contain a slash.
146+
# The repo mapping should not be applied to single segment paths, which may
147+
# be root symlinks.
148+
local -r remainder=$(echo "$1" | cut -s -d / -f 2-)
146149
if [[ -n "$remainder" ]]; then
147150
if [[ -z "${2+x}" ]]; then
148151
local -r source_repo=$(runfiles_current_repository 2)

‎tools/bash/runfiles/runfiles_test.bash

+8
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,12 @@ function test_directory_based_runfiles_with_repo_mapping_from_main() {
216216
export RUNFILES_DIR=${tmpdir}/mock/runfiles
217217
mkdir -p "$RUNFILES_DIR"
218218
cat > "$RUNFILES_DIR/_repo_mapping" <<EOF
219+
,config.json,config.json~1.2.3
219220
,my_module,_main
220221
,my_protobuf,protobuf~3.19.2
221222
,my_workspace,_main
222223
protobuf~3.19.2,protobuf,protobuf~3.19.2
224+
protobuf~3.19.2,config.json,config.json~1.2.3
223225
EOF
224226
export RUNFILES_MANIFEST_FILE=
225227
source "$runfiles_lib_path"
@@ -258,10 +260,12 @@ function test_directory_based_runfiles_with_repo_mapping_from_other_repo() {
258260
export RUNFILES_DIR=${tmpdir}/mock/runfiles
259261
mkdir -p "$RUNFILES_DIR"
260262
cat > "$RUNFILES_DIR/_repo_mapping" <<EOF
263+
,config.json,config.json~1.2.3
261264
,my_module,_main
262265
,my_protobuf,protobuf~3.19.2
263266
,my_workspace,_main
264267
protobuf~3.19.2,protobuf,protobuf~3.19.2
268+
protobuf~3.19.2,config.json,config.json~1.2.3
265269
EOF
266270
export RUNFILES_MANIFEST_FILE=
267271
source "$runfiles_lib_path"
@@ -296,10 +300,12 @@ function test_manifest_based_runfiles_with_repo_mapping_from_main() {
296300
local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)"
297301

298302
cat > "$tmpdir/foo.repo_mapping" <<EOF
303+
,config.json,config.json~1.2.3
299304
,my_module,_main
300305
,my_protobuf,protobuf~3.19.2
301306
,my_workspace,_main
302307
protobuf~3.19.2,protobuf,protobuf~3.19.2
308+
protobuf~3.19.2,config.json,config.json~1.2.3
303309
EOF
304310
export RUNFILES_DIR=
305311
export RUNFILES_MANIFEST_FILE="$tmpdir/foo.runfiles_manifest"
@@ -344,10 +350,12 @@ function test_manifest_based_runfiles_with_repo_mapping_from_other_repo() {
344350
local tmpdir="$(mktemp -d $TEST_TMPDIR/tmp.XXXXXXXX)"
345351

346352
cat > "$tmpdir/foo.repo_mapping" <<EOF
353+
,config.json,config.json~1.2.3
347354
,my_module,_main
348355
,my_protobuf,protobuf~3.19.2
349356
,my_workspace,_main
350357
protobuf~3.19.2,protobuf,protobuf~3.19.2
358+
protobuf~3.19.2,config.json,config.json~1.2.3
351359
EOF
352360
export RUNFILES_DIR=
353361
export RUNFILES_MANIFEST_FILE="$tmpdir/foo.runfiles_manifest"

‎tools/cpp/runfiles/runfiles_test.cc

+30-20
Original file line numberDiff line numberDiff line change
@@ -586,10 +586,12 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
586586

587587
TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromMain) {
588588
string uid = LINE_AS_STRING();
589-
unique_ptr<MockFile> rm(MockFile::Create(
590-
"foo" + uid + ".repo_mapping",
591-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
592-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
589+
unique_ptr<MockFile> rm(
590+
MockFile::Create("foo" + uid + ".repo_mapping",
591+
{",config.json,config.json~1.2.3", ",my_module,_main",
592+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
593+
"protobuf~3.19.2,config.json,config.json~1.2.3",
594+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
593595
ASSERT_TRUE(rm != nullptr);
594596
unique_ptr<MockFile> mf(MockFile::Create(
595597
"foo" + uid + ".runfiles_manifest",
@@ -644,10 +646,12 @@ TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromMain) {
644646

645647
TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) {
646648
string uid = LINE_AS_STRING();
647-
unique_ptr<MockFile> rm(MockFile::Create(
648-
"foo" + uid + ".repo_mapping",
649-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
650-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
649+
unique_ptr<MockFile> rm(
650+
MockFile::Create("foo" + uid + ".repo_mapping",
651+
{",config.json,config.json~1.2.3", ",my_module,_main",
652+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
653+
"protobuf~3.19.2,config.json,config.json~1.2.3",
654+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
651655
ASSERT_TRUE(rm != nullptr);
652656
unique_ptr<MockFile> mf(MockFile::Create(
653657
"foo" + uid + ".runfiles_manifest",
@@ -699,10 +703,12 @@ TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) {
699703

700704
TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) {
701705
string uid = LINE_AS_STRING();
702-
unique_ptr<MockFile> rm(MockFile::Create(
703-
"foo" + uid + ".runfiles/_repo_mapping",
704-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
705-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
706+
unique_ptr<MockFile> rm(
707+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
708+
{",config.json,config.json~1.2.3", ",my_module,_main",
709+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
710+
"protobuf~3.19.2,config.json,config.json~1.2.3",
711+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
706712
ASSERT_TRUE(rm != nullptr);
707713
string dir = rm->DirName();
708714
string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));
@@ -746,10 +752,12 @@ TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) {
746752

747753
TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromOtherRepo) {
748754
string uid = LINE_AS_STRING();
749-
unique_ptr<MockFile> rm(MockFile::Create(
750-
"foo" + uid + ".runfiles/_repo_mapping",
751-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
752-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
755+
unique_ptr<MockFile> rm(
756+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
757+
{",config.json,config.json~1.2.3", ",my_module,_main",
758+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
759+
"protobuf~3.19.2,config.json,config.json~1.2.3",
760+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
753761
ASSERT_TRUE(rm != nullptr);
754762
string dir = rm->DirName();
755763
string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));
@@ -790,10 +798,12 @@ TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromOtherRepo) {
790798
TEST_F(RunfilesTest,
791799
DirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepo) {
792800
string uid = LINE_AS_STRING();
793-
unique_ptr<MockFile> rm(MockFile::Create(
794-
"foo" + uid + ".runfiles/_repo_mapping",
795-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
796-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
801+
unique_ptr<MockFile> rm(
802+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
803+
{",config.json,config.json~1.2.3", ",my_module,_main",
804+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
805+
"protobuf~3.19.2,config.json,config.json~1.2.3",
806+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
797807
ASSERT_TRUE(rm != nullptr);
798808
string dir = rm->DirName();
799809
string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));

‎tools/java/runfiles/testing/RunfilesTest.java

+12
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,11 @@ public void testManifestBasedRlocationWithRepoMapping_fromMain() throws Exceptio
264264
tempFile(
265265
"foo.repo_mapping",
266266
ImmutableList.of(
267+
",config.json,config.json~1.2.3",
267268
",my_module,_main",
268269
",my_protobuf,protobuf~3.19.2",
269270
",my_workspace,_main",
271+
"protobuf~3.19.2,config.json,config.json~1.2.3",
270272
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
271273
Path mf =
272274
tempFile(
@@ -318,9 +320,11 @@ public void testManifestBasedRlocationUnmapped() throws Exception {
318320
tempFile(
319321
"foo.repo_mapping",
320322
ImmutableList.of(
323+
",config.json,config.json~1.2.3",
321324
",my_module,_main",
322325
",my_protobuf,protobuf~3.19.2",
323326
",my_workspace,_main",
327+
"protobuf~3.19.2,config.json,config.json~1.2.3",
324328
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
325329
Path mf =
326330
tempFile(
@@ -367,9 +371,11 @@ public void testManifestBasedRlocationWithRepoMapping_fromOtherRepo() throws Exc
367371
tempFile(
368372
"foo.repo_mapping",
369373
ImmutableList.of(
374+
",config.json,config.json~1.2.3",
370375
",my_module,_main",
371376
",my_protobuf,protobuf~3.19.2",
372377
",my_workspace,_main",
378+
"protobuf~3.19.2,config.json,config.json~1.2.3",
373379
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
374380
Path mf =
375381
tempFile(
@@ -419,9 +425,11 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromMain() throws Excepti
419425
tempFile(
420426
dir.resolve("_repo_mapping").toString(),
421427
ImmutableList.of(
428+
",config.json,config.json~1.2.3",
422429
",my_module,_main",
423430
",my_protobuf,protobuf~3.19.2",
424431
",my_workspace,_main",
432+
"protobuf~3.19.2,config.json,config.json~1.2.3",
425433
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
426434
Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()).withSourceRepository("");
427435

@@ -458,9 +466,11 @@ public void testDirectoryBasedRlocationUnmapped() throws Exception {
458466
tempFile(
459467
dir.resolve("_repo_mapping").toString(),
460468
ImmutableList.of(
469+
",config.json,config.json~1.2.3",
461470
",my_module,_main",
462471
",my_protobuf,protobuf~3.19.2",
463472
",my_workspace,_main",
473+
"protobuf~3.19.2,config.json,config.json~1.2.3",
464474
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
465475
Runfiles r = Runfiles.createDirectoryBasedForTesting(dir.toString()).unmapped();
466476

@@ -497,9 +507,11 @@ public void testDirectoryBasedRlocationWithRepoMapping_fromOtherRepo() throws Ex
497507
tempFile(
498508
dir.resolve("_repo_mapping").toString(),
499509
ImmutableList.of(
510+
",config.json,config.json~1.2.3",
500511
",my_module,_main",
501512
",my_protobuf,protobuf~3.19.2",
502513
",my_workspace,_main",
514+
"protobuf~3.19.2,config.json,config.json~1.2.3",
503515
"protobuf~3.19.2,protobuf,protobuf~3.19.2"));
504516
Runfiles r =
505517
Runfiles.createDirectoryBasedForTesting(dir.toString())

0 commit comments

Comments
 (0)