Skip to content

Commit e895ff0

Browse files
fmeumfmeum
authored
Test and fix root symlink edge case in runfiles library (bazelbuild#17365)
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 40d83a7 commit e895ff0

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
@@ -582,10 +582,12 @@ TEST_F(RunfilesTest, PathsFromEnvVars) {
582582

583583
TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromMain) {
584584
string uid = LINE_AS_STRING();
585-
unique_ptr<MockFile> rm(MockFile::Create(
586-
"foo" + uid + ".repo_mapping",
587-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
588-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
585+
unique_ptr<MockFile> rm(
586+
MockFile::Create("foo" + uid + ".repo_mapping",
587+
{",config.json,config.json~1.2.3", ",my_module,_main",
588+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
589+
"protobuf~3.19.2,config.json,config.json~1.2.3",
590+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
589591
ASSERT_TRUE(rm != nullptr);
590592
unique_ptr<MockFile> mf(MockFile::Create(
591593
"foo" + uid + ".runfiles_manifest",
@@ -638,10 +640,12 @@ TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromMain) {
638640

639641
TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) {
640642
string uid = LINE_AS_STRING();
641-
unique_ptr<MockFile> rm(MockFile::Create(
642-
"foo" + uid + ".repo_mapping",
643-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
644-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
643+
unique_ptr<MockFile> rm(
644+
MockFile::Create("foo" + uid + ".repo_mapping",
645+
{",config.json,config.json~1.2.3", ",my_module,_main",
646+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
647+
"protobuf~3.19.2,config.json,config.json~1.2.3",
648+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
645649
ASSERT_TRUE(rm != nullptr);
646650
unique_ptr<MockFile> mf(MockFile::Create(
647651
"foo" + uid + ".runfiles_manifest",
@@ -692,10 +696,12 @@ TEST_F(RunfilesTest, ManifestBasedRlocationWithRepoMapping_fromOtherRepo) {
692696

693697
TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) {
694698
string uid = LINE_AS_STRING();
695-
unique_ptr<MockFile> rm(MockFile::Create(
696-
"foo" + uid + ".runfiles/_repo_mapping",
697-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
698-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
699+
unique_ptr<MockFile> rm(
700+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
701+
{",config.json,config.json~1.2.3", ",my_module,_main",
702+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
703+
"protobuf~3.19.2,config.json,config.json~1.2.3",
704+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
699705
ASSERT_TRUE(rm != nullptr);
700706
string dir = rm->DirName();
701707
string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));
@@ -737,10 +743,12 @@ TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromMain) {
737743

738744
TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromOtherRepo) {
739745
string uid = LINE_AS_STRING();
740-
unique_ptr<MockFile> rm(MockFile::Create(
741-
"foo" + uid + ".runfiles/_repo_mapping",
742-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
743-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
746+
unique_ptr<MockFile> rm(
747+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
748+
{",config.json,config.json~1.2.3", ",my_module,_main",
749+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
750+
"protobuf~3.19.2,config.json,config.json~1.2.3",
751+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
744752
ASSERT_TRUE(rm != nullptr);
745753
string dir = rm->DirName();
746754
string argv0(dir.substr(0, dir.size() - string(".runfiles").size()));
@@ -780,10 +788,12 @@ TEST_F(RunfilesTest, DirectoryBasedRlocationWithRepoMapping_fromOtherRepo) {
780788
TEST_F(RunfilesTest,
781789
DirectoryBasedRlocationWithRepoMapping_fromOtherRepo_withSourceRepo) {
782790
string uid = LINE_AS_STRING();
783-
unique_ptr<MockFile> rm(MockFile::Create(
784-
"foo" + uid + ".runfiles/_repo_mapping",
785-
{",my_module,_main", ",my_protobuf,protobuf~3.19.2",
786-
",my_workspace,_main", "protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
791+
unique_ptr<MockFile> rm(
792+
MockFile::Create("foo" + uid + ".runfiles/_repo_mapping",
793+
{",config.json,config.json~1.2.3", ",my_module,_main",
794+
",my_protobuf,protobuf~3.19.2", ",my_workspace,_main",
795+
"protobuf~3.19.2,config.json,config.json~1.2.3",
796+
"protobuf~3.19.2,protobuf,protobuf~3.19.2"}));
787797
ASSERT_TRUE(rm != nullptr);
788798
string dir = rm->DirName();
789799
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)