diff options
10 files changed, 215 insertions, 14 deletions
diff --git a/doc/concepts/overview.md b/doc/concepts/overview.md index a9bcc847..18552f87 100644 --- a/doc/concepts/overview.md +++ b/doc/concepts/overview.md @@ -136,6 +136,11 @@ target files. second position (where normally the module would be) is necessary to ensure the name has length more than 2 to distinguish it from a reference to the module `"FILE"`. + - An explicit reference of a non-upwards symlink target in the same module, + specified as `["SYMLINK", null, name]`. The explicit `null` at the + second position is required for the same reason as in the explicit + file reference. It is the user's responsibility to ensure the symlink + pointed to is non-upwards. - A reference to an collection, given by a shell pattern, of explicit source files in the top-level directory of the same module, specified as `["GLOB", null, pattern]`. The explicit `null` at @@ -188,6 +193,18 @@ the relative path of the file to the module root and the value the file artifact located at the specified location. The runfiles are the same as the artifacts and the provides map is empty. +#### (Non-upwards) Symlinks + +To ensure self-containedness and location-independence, only +*non-upwards* symlinks are expected and accepted. The symlinks +must not however be necessarily resolvable, i.e., dangling symlinks +are accepted. + +An explicit (non-upwards) symlink target is similar to an explicit file target, +except that at the specified location there has to be a non-upwards symlink +rather than a file and the corresponding symlink artifact is taken instead of a +file artifact. + #### Collection of files given by a shell pattern A collection of files given by a shell pattern has, both as diff --git a/src/buildtool/build_engine/base_maps/entity_name.hpp b/src/buildtool/build_engine/base_maps/entity_name.hpp index e2df632e..bec5b083 100644 --- a/src/buildtool/build_engine/base_maps/entity_name.hpp +++ b/src/buildtool/build_engine/base_maps/entity_name.hpp @@ -112,11 +112,13 @@ template <typename T> std::nullopt) noexcept -> std::optional<EntityName> { try { bool const is_file = s0 == EntityName::kFileLocationMarker; + bool const is_glob = s0 == EntityName::kGlobMarker; + bool const is_symlink = s0 == EntityName::kSymlinkLocationMarker; auto const ref_type = - s0 == EntityName::kFileLocationMarker - ? ReferenceType::kFile - : (s0 == EntityName::kGlobMarker ? ReferenceType::kGlob - : ReferenceType::kTree); + is_file ? ReferenceType::kFile + : (is_glob ? ReferenceType::kGlob + : (is_symlink ? ReferenceType::kSymlink + : ReferenceType::kTree)); if (list_size == 3) { if (IsString(list[2])) { auto const& name = GetString(list[2]); @@ -232,7 +234,8 @@ template <typename T> } else if (s0 == EntityName::kFileLocationMarker or s0 == EntityName::kTreeLocationMarker or - s0 == EntityName::kGlobMarker) { + s0 == EntityName::kGlobMarker or + s0 == EntityName::kSymlinkLocationMarker) { return ParseEntityNameFSReference( s0, list, list_size, current, logger); } diff --git a/src/buildtool/build_engine/base_maps/entity_name_data.hpp b/src/buildtool/build_engine/base_maps/entity_name_data.hpp index c44be2b3..0de1b48f 100644 --- a/src/buildtool/build_engine/base_maps/entity_name_data.hpp +++ b/src/buildtool/build_engine/base_maps/entity_name_data.hpp @@ -38,7 +38,13 @@ struct AnonymousTarget { } }; -enum class ReferenceType : std::int8_t { kTarget, kFile, kTree, kGlob }; +enum class ReferenceType : std::int8_t { + kTarget, + kFile, + kTree, + kGlob, + kSymlink +}; struct NamedTarget { std::string repository{}; @@ -80,6 +86,7 @@ class EntityName { static constexpr auto kFileLocationMarker = "FILE"; static constexpr auto kTreeLocationMarker = "TREE"; static constexpr auto kGlobMarker = "GLOB"; + static constexpr auto kSymlinkLocationMarker = "SYMLINK"; static constexpr auto kRelativeLocationMarker = "./"; static constexpr auto kAnonymousMarker = "#"; @@ -137,6 +144,9 @@ class EntityName { else if (x.reference_t == ReferenceType::kGlob) { j.push_back(kGlobMarker); } + else if (x.reference_t == ReferenceType::kSymlink) { + j.push_back(kSymlinkLocationMarker); + } j.push_back(x.module); j.push_back(x.name); } diff --git a/src/buildtool/build_engine/target_map/target_map.cpp b/src/buildtool/build_engine/target_map/target_map.cpp index d2e73f07..99f6d9f0 100644 --- a/src/buildtool/build_engine/target_map/target_map.cpp +++ b/src/buildtool/build_engine/target_map/target_map.cpp @@ -1471,6 +1471,17 @@ void TreeTarget( .config = Configuration{}}); } + for (const auto& x : dir_entries.SymlinksIterator()) { + v.emplace_back(ConfiguredTarget{ + .target = + BuildMaps::Base::EntityName{ + target.repository, + dir_name, + x, + BuildMaps::Base::ReferenceType::kSymlink}, + .config = Configuration{}}); + } + for (const auto& x : dir_entries.DirectoriesIterator()) { v.emplace_back(ConfiguredTarget{ .target = @@ -1573,6 +1584,15 @@ void GlobTargetWithDirEntry( BuildMaps::Base::ReferenceType::kFile}); } } + for (auto const& x : dir.SymlinksIterator()) { + if (fnmatch(pattern.c_str(), x.c_str(), 0) == 0) { + matches.emplace_back(BuildMaps::Base::EntityName{ + target.repository, + target.module, + x, + BuildMaps::Base::ReferenceType::kSymlink}); + } + } source_target_map->ConsumeAfterKeysReady( ts, matches, @@ -1645,6 +1665,23 @@ auto CreateTargetMap( }); } else if (key.target.GetNamedTarget().reference_t == + BuildMaps::Base::ReferenceType::kSymlink) { + // Not a defined target, treat as source target + source_target_map->ConsumeAfterKeysReady( + ts, + {key.target}, + [setter](auto values) { + (*setter)(AnalysedTargetPtr{*values[0]}); + }, + [logger, target = key.target](auto const& msg, auto fatal) { + (*logger)(fmt::format("While analysing target {} as " + "symlink:\n{}", + target.ToString(), + msg), + fatal); + }); + } + else if (key.target.GetNamedTarget().reference_t == BuildMaps::Base::ReferenceType::kGlob) { auto wrapped_logger = std::make_shared<AsyncMapConsumerLogger>( [logger, target = key.target](auto const& msg, bool fatal) { diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp index 288101f5..5589b0e0 100644 --- a/src/buildtool/main/main.cpp +++ b/src/buildtool/main/main.cpp @@ -1238,6 +1238,9 @@ auto DetermineNonExplicitTarget( case Base::ReferenceType::kGlob: std::cout << id.ToString() << " is a glob." << std::endl; return std::nullopt; + case Base::ReferenceType::kSymlink: + std::cout << id.ToString() << " is a symlink." << std::endl; + return std::nullopt; case Base::ReferenceType::kTarget: return id; } diff --git a/test/buildtool/build_engine/base_maps/source_map.test.cpp b/test/buildtool/build_engine/base_maps/source_map.test.cpp index 3b787ffe..05157870 100644 --- a/test/buildtool/build_engine/base_maps/source_map.test.cpp +++ b/test/buildtool/build_engine/base_maps/source_map.test.cpp @@ -28,6 +28,14 @@ namespace { using namespace BuildMaps::Base; // NOLINT void SetupConfig(bool use_git) { + // manually create locally a test symlink in data_src; should match the + // git test_repo structure + if (not use_git) { + auto link_path = kBasePath / "data_src/foo/link"; + if (not FileSystemManager::Exists(link_path)) { + REQUIRE(FileSystemManager::CreateSymlink("dummy", link_path)); + } + } auto root = FileRoot{kBasePath / "data_src"}; if (use_git) { auto repo_path = CreateTestRepo(); @@ -156,3 +164,24 @@ TEST_CASE("subdir file") { CHECK(artifacts["bar/file"]["data"]["size"] == 0); } } + +TEST_CASE("subdir symlink") { + nlohmann::json artifacts; + auto name = EntityName{"", "foo", "link"}; + auto consumer = [&artifacts](auto values) { + artifacts = (*values[0])->Artifacts()->ToJson(); + }; + + SECTION("via file") { + CHECK(ReadSourceTarget(name, consumer, /*use_git=*/false)); + CHECK(artifacts["link"]["type"] == "LOCAL"); + CHECK(artifacts["link"]["data"]["path"] == "foo/link"); + } + + SECTION("via git tree") { + CHECK(ReadSourceTarget(name, consumer, /*use_git=*/true)); + CHECK(artifacts["link"]["type"] == "KNOWN"); + CHECK(artifacts["link"]["data"]["id"] == kSrcLinkId); + CHECK(artifacts["link"]["data"]["size"] == 5); // content: dummy + } +} diff --git a/test/buildtool/build_engine/target_map/TARGETS b/test/buildtool/build_engine/target_map/TARGETS index 7eae3c0c..e83c22d1 100644 --- a/test/buildtool/build_engine/target_map/TARGETS +++ b/test/buildtool/build_engine/target_map/TARGETS @@ -67,6 +67,7 @@ , "data_targets/result/TARGETS" , "data_targets/simple_rules/TARGETS" , "data_targets/simple_targets/TARGETS" + , "data_targets/symlink_reference/TARGETS" , "data_targets/tree/TARGETS" , "data_targets/x/TARGETS" , "data_targets/x/x/TARGETS" diff --git a/test/buildtool/build_engine/target_map/data_targets/simple_targets/TARGETS b/test/buildtool/build_engine/target_map/data_targets/simple_targets/TARGETS index 4306c3ae..0001a223 100644 --- a/test/buildtool/build_engine/target_map/data_targets/simple_targets/TARGETS +++ b/test/buildtool/build_engine/target_map/data_targets/simple_targets/TARGETS @@ -7,11 +7,11 @@ } , "collect dep artifacts": { "type": ["simple_rules", "collect deps"] - , "deps": ["foo.txt", "bar.txt", "baz.txt"] + , "deps": ["foo.txt", "bar.txt", "baz.txt", "link"] } , "collect as runfiles": { "type": ["simple_rules", "collect deps as runfiles"] - , "deps": ["foo.txt", "bar.txt", "baz.txt"] + , "deps": ["foo.txt", "bar.txt", "baz.txt", "link"] } , "stage blob": { "type": ["simple_rules", "text file"] @@ -43,16 +43,26 @@ ] , "analyze": ["collect as runfiles"] } +, "use generic sym": + { "type": "generic" + , "deps": ["bar.txt"] + , "cmds": ["ln -s $(cat bar.txt) sym"] + , "outs": ["sym"] + } , "use generic": { "type": "generic" - , "deps": ["foo.txt", "bar.txt"] - , "cmds": ["cat foo.txt bar.txt > out", "echo 'DONE' >> out"] + , "deps": ["foo.txt", "link"] + , "cmds": ["cat foo.txt > out", "readlink link > out", "echo 'DONE' >> out"] , "outs": ["out"] } , "install": { "type": "install" - , "deps": ["foo.txt", "bar.txt"] - , "files": {"combined.txt": "use generic", "subdir/restaged.txt": "bar.txt"} + , "deps": ["foo.txt", "bar.txt", "link"] + , "files": + { "link_gen": "use generic sym" + , "combined.txt": "use generic" + , "subdir/restaged.txt": "bar.txt" + } , "dirs": [ ["collect as runfiles", "mix/in/this/subdir"] , ["runfile names", "mix/in/this/subdir"] diff --git a/test/buildtool/build_engine/target_map/data_targets/symlink_reference/TARGETS b/test/buildtool/build_engine/target_map/data_targets/symlink_reference/TARGETS new file mode 100644 index 00000000..c05d9517 --- /dev/null +++ b/test/buildtool/build_engine/target_map/data_targets/symlink_reference/TARGETS @@ -0,0 +1,9 @@ +{ "_link": + {"type": "install", "files": {"raw_data/link": ["SYMLINK", null, "link"]}} +, "link": + { "type": "generic" + , "deps": ["_link"] + , "outs": ["link"] + , "cmds": ["ln -s $(readlink raw_data/link | tr 'a-z' 'A-Z') link"] + } +} diff --git a/test/buildtool/build_engine/target_map/target_map.test.cpp b/test/buildtool/build_engine/target_map/target_map.test.cpp index 9b667425..98d3a938 100644 --- a/test/buildtool/build_engine/target_map/target_map.test.cpp +++ b/test/buildtool/build_engine/target_map/target_map.test.cpp @@ -28,7 +28,24 @@ namespace { using none_t = Expression::none_t; +auto CreateSymlinks() -> bool { + auto base_src = std::filesystem::path( + "test/buildtool/build_engine/target_map/data_src"); + // create the symlinks + REQUIRE(FileSystemManager::CreateSymlink( + "dummy", base_src / "a" / "b" / "targets_here" / "c" / "d" / "link")); + REQUIRE(FileSystemManager::CreateSymlink( + "dummy", base_src / "symlink_reference" / "link")); + REQUIRE(FileSystemManager::CreateSymlink( + "dummy", base_src / "simple_targets" / "link")); + + return true; +} + void SetupConfig() { + // manually create locally test symlinks in data_src, but only once + [[maybe_unused]] static auto done = CreateSymlinks(); + // create the file roots auto info = RepositoryConfig::RepositoryInfo{ FileRoot{"test/buildtool/build_engine/target_map/data_src"}, FileRoot{"test/buildtool/build_engine/target_map/data_targets"}, @@ -85,6 +102,32 @@ TEST_CASE("simple targets") { CHECK(artifact->IsArtifact()); } + SECTION("Actual source symlink file") { + { + error_msg = "NONE"; + error = false; + + TaskSystem ts; + target_map.ConsumeAfterKeysReady( + &ts, + {BuildMaps::Target::ConfiguredTarget{ + .target = + BuildMaps::Base::EntityName{ + "", "a/b/targets_here", "c/d/link"}, + .config = empty_config}}, + [&result](auto values) { result = *values[0]; }, + [&error, &error_msg](std::string const& msg, bool /*unused*/) { + error = true; + error_msg = msg; + }); + } + CHECK(!error); + CHECK(error_msg == "NONE"); + auto artifacts = result->Artifacts(); + ExpressionPtr artifact = artifacts->Get("c/d/link", none_t{}); + CHECK(artifact->IsArtifact()); + } + SECTION("No targets file here") { { error_msg = "NONE"; @@ -248,6 +291,7 @@ TEST_CASE("simple targets") { "simple_targets/bar.txt"); CHECK(artifacts_desc["baz.txt"]["data"]["path"] == "simple_targets/baz.txt"); + CHECK(artifacts_desc["link"]["data"]["path"] == "simple_targets/link"); } SECTION("Rule stages blob") { @@ -441,7 +485,7 @@ TEST_CASE("generator functions in string arguments") { CHECK(!error); CHECK(error_msg == "NONE"); CHECK(result->Artifacts()->ToJson()["index.txt"]["type"] == "KNOWN"); - CHECK(result->Blobs()[0] == "bar.txt;baz.txt;foo.txt"); + CHECK(result->Blobs()[0] == "bar.txt;baz.txt;foo.txt;link"); } SECTION("runfies") { @@ -465,7 +509,7 @@ TEST_CASE("generator functions in string arguments") { CHECK(!error); CHECK(error_msg == "NONE"); CHECK(result->Artifacts()->ToJson()["index.txt"]["type"] == "KNOWN"); - CHECK(result->Blobs()[0] == "bar.txt;baz.txt;foo.txt"); + CHECK(result->Blobs()[0] == "bar.txt;baz.txt;foo.txt;link"); } } @@ -539,8 +583,12 @@ TEST_CASE("built-in rules") { CHECK(stage["foo.txt"]["data"]["path"] == "simple_targets/foo.txt"); CHECK(stage["bar.txt"]["type"] == "LOCAL"); CHECK(stage["bar.txt"]["data"]["path"] == "simple_targets/bar.txt"); + CHECK(stage["bar.txt"]["type"] == "LOCAL"); + CHECK(stage["link"]["data"]["path"] == "simple_targets/link"); CHECK(stage["combined.txt"]["type"] == "ACTION"); CHECK(stage["combined.txt"]["data"]["path"] == "out"); + CHECK(stage["link_gen"]["type"] == "ACTION"); + CHECK(stage["link_gen"]["data"]["path"] == "sym"); CHECK(stage["subdir/restaged.txt"]["type"] == "LOCAL"); CHECK(stage["subdir/restaged.txt"]["data"]["path"] == "simple_targets/bar.txt"); @@ -550,6 +598,8 @@ TEST_CASE("built-in rules") { "simple_targets/bar.txt"); CHECK(stage["mix/in/this/subdir/baz.txt"]["data"]["path"] == "simple_targets/baz.txt"); + CHECK(stage["mix/in/this/subdir/link"]["data"]["path"] == + "simple_targets/link"); CHECK(stage["mix/in/this/subdir/index.txt"]["type"] == "KNOWN"); } @@ -667,6 +717,38 @@ TEST_CASE("target reference") { "file_reference/hello.txt"); } + SECTION("symlink vs target") { + error = false; + error_msg = "NONE"; + { + TaskSystem ts; + target_map.ConsumeAfterKeysReady( + &ts, + {BuildMaps::Target::ConfiguredTarget{ + .target = + BuildMaps::Base::EntityName{ + "", "symlink_reference", "link"}, + .config = empty_config}}, + [&result](auto values) { result = *values[0]; }, + [&error, &error_msg](std::string const& msg, bool /*unused*/) { + error = true; + error_msg = msg; + }); + } + CHECK(!error); + CHECK(error_msg == "NONE"); + CHECK(result->Artifacts()->ToJson()["link"]["type"] == "ACTION"); + CHECK(result->Artifacts()->ToJson()["link"]["data"]["path"] == "link"); + + CHECK(result->Actions().size() == 1); + CHECK( + result->Actions()[0]->ToJson()["input"]["raw_data/link"]["type"] == + "LOCAL"); + CHECK(result->Actions()[0] + ->ToJson()["input"]["raw_data/link"]["data"]["path"] == + "symlink_reference/link"); + } + SECTION("relative address") { error = false; error_msg = "NONE"; |