From 98dd5076db7ecf2eb1e22252144091bde7b8a529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Wed, 29 Jun 2022 17:39:35 +0200 Subject: [PATCH 1/5] Revert determineVersionWithGitTool to the behavior before #2263. --- source/dub/internal/git.d | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/source/dub/internal/git.d b/source/dub/internal/git.d index d7e169f18..24e3e7bce 100644 --- a/source/dub/internal/git.d +++ b/source/dub/internal/git.d @@ -103,10 +103,7 @@ private string determineVersionFromGitDescribe(string describeOutput) if (tag.startsWith("v") && isValidVersion(tag[1 .. $])) { if (num == 0) return tag[1 .. $]; const i = tag.indexOf('+'); - auto r = format("%s%scommit.%s.%s", tag[1 .. (i < 0) ? $ : i], - parts.length > 3 ? "." : "-", num, commit); - if (i > 0) r ~= tag[i .. $]; - return r; + return format("%s%scommit.%s.%s", tag[1 .. $], i >= 0 ? '.' : '+', num, commit); } return null; } @@ -115,19 +112,19 @@ unittest { // tag v1.0.0 assert(determineVersionFromGitDescribe("v1.0.0-0-deadbeef") == "1.0.0"); // 1 commit after v1.0.0 - assert(determineVersionFromGitDescribe("v1.0.0-1-deadbeef") == "1.0.0-commit.1.deadbeef"); + assert(determineVersionFromGitDescribe("v1.0.0-1-deadbeef") == "1.0.0+commit.1.deadbeef"); // tag v1.0.0+2.0.0 assert(determineVersionFromGitDescribe("v1.0.0+2.0.0-0-deadbeef") == "1.0.0+2.0.0"); // 12 commits after tag v1.0.0+2.0.0 - assert(determineVersionFromGitDescribe("v1.0.0+2.0.0-12-deadbeef") == "1.0.0-commit.12.deadbeef+2.0.0"); + assert(determineVersionFromGitDescribe("v1.0.0+2.0.0-12-deadbeef") == "1.0.0+2.0.0.commit.12.deadbeef"); // tag v1.0.0-beta.1 assert(determineVersionFromGitDescribe("v1.0.0-beta.1-0-deadbeef") == "1.0.0-beta.1"); // 2 commits after tag v1.0.0-beta.1 - assert(determineVersionFromGitDescribe("v1.0.0-beta.1-2-deadbeef") == "1.0.0-beta.1.commit.2.deadbeef"); + assert(determineVersionFromGitDescribe("v1.0.0-beta.1-2-deadbeef") == "1.0.0-beta.1+commit.2.deadbeef"); // tag v1.0.0-beta.2+2.0.0 assert(determineVersionFromGitDescribe("v1.0.0-beta.2+2.0.0-0-deadbeef") == "1.0.0-beta.2+2.0.0"); // 3 commits after tag v1.0.0-beta.2+2.0.0 - assert(determineVersionFromGitDescribe("v1.0.0-beta.2+2.0.0-3-deadbeef") == "1.0.0-beta.2.commit.3.deadbeef+2.0.0"); + assert(determineVersionFromGitDescribe("v1.0.0-beta.2+2.0.0-3-deadbeef") == "1.0.0-beta.2+2.0.0.commit.3.deadbeef"); // invalid tags assert(determineVersionFromGitDescribe("1.0.0-0-deadbeef") is null); From 0c6361099fc3e6cb80d1244e1f4d21d8597b420a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Mon, 4 Jul 2022 12:06:08 +0200 Subject: [PATCH 2/5] Match versions of cached packages exactly. Uses exact version matching instead of SemVer matching for cached packages to enforce exact matches for SCM based dependencies in particular. --- source/dub/dependency.d | 23 +++++++++++++++++++++++ source/dub/packagemanager.d | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/source/dub/dependency.d b/source/dub/dependency.d index 52dc3ee8c..6ff7dbd83 100644 --- a/source/dub/dependency.d +++ b/source/dub/dependency.d @@ -863,6 +863,19 @@ struct Version { /// Tests if this represents the special unknown version constant. @property bool isUnknown() const { return m_version == UNKNOWN_VERS; } + /** Tests two versions for equality, according to the selected match mode. + */ + bool matches(Version other, VersionMatchMode mode = VersionMatchMode.standard) + const { + if (this != other) + return false; + + if (mode == VersionMatchMode.strict && this.toString() != other.toString()) + return false; + + return true; + } + /** Compares two versions/branches for precedence. Versions generally have precedence over branches and the master branch @@ -900,6 +913,11 @@ struct Version { string toString() const { return m_version; } } +enum VersionMatchMode { + standard, /// Match according to SemVer rules + strict /// Also include build metadata suffix in the comparison +} + unittest { Version a, b; @@ -965,6 +983,11 @@ unittest { assert(Version("1.0.0+a") == Version("1.0.0+b")); assert(Version("73535568b79a0b124bc1653002637a830ce0fcb8").isSCM); + + assert(Version("1.0.0").matches(Version("1.0.0+foo"))); + assert(Version("1.0.0").matches(Version("1.0.0+foo"), VersionMatchMode.standard)); + assert(!Version("1.0.0").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); + assert(Version("1.0.0+foo").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); } /// Determines whether the given string is a Git hash. diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index cb637bda7..c0a95333e 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -173,7 +173,7 @@ class PackageManager { } foreach (p; getPackageIterator(name)) - if (p.version_ == ver) + if (p.version_.matches(ver, isManagedPackage(p) ? VersionMatchMode.strict : VersionMatchMode.standard)) return p; return null; From c4b05a1735fc9303d7cf1f7dfb9498e5ed4f102f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Mon, 4 Jul 2022 17:17:34 +0200 Subject: [PATCH 3/5] Also let getBestPackage use VersionMatchMode.strict for managed packages. --- source/dub/dependency.d | 16 +++++++++++++--- source/dub/packagemanager.d | 6 ++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/source/dub/dependency.d b/source/dub/dependency.d index 6ff7dbd83..0fedb975b 100644 --- a/source/dub/dependency.d +++ b/source/dub/dependency.d @@ -475,11 +475,11 @@ struct Dependency { /** Tests if the specification matches a specific version. */ - bool matches(string vers) const { return matches(Version(vers)); } + bool matches(string vers, VersionMatchMode mode = VersionMatchMode.standard) const { return matches(Version(vers), mode); } /// ditto - bool matches(const(Version) v) const { return matches(v); } + bool matches(const(Version) v, VersionMatchMode mode = VersionMatchMode.standard) const { return matches(v, mode); } /// ditto - bool matches(ref const(Version) v) const { + bool matches(ref const(Version) v, VersionMatchMode mode = VersionMatchMode.standard) const { if (this.matchesAny) return true; if (this.isSCM) return true; //logDebug(" try match: %s with: %s", v, this); @@ -494,6 +494,9 @@ struct Dependency { return false; if( !doCmp(m_inclusiveB, v, m_versB) ) return false; + if (this.isExactVersion && mode == VersionMatchMode.strict + && this.version_.toString != v.toString) + return false; return true; } @@ -713,6 +716,13 @@ unittest { assert(a.merge(b) == b); assert(b.merge(a) == b); + assert(Dependency("1.0.0").matches(Version("1.0.0+foo"))); + assert(Dependency("1.0.0").matches(Version("1.0.0+foo"), VersionMatchMode.standard)); + assert(!Dependency("1.0.0").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); + assert(Dependency("1.0.0+foo").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); + assert(Dependency("~>1.0.0+foo").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); + assert(Dependency("~>1.0.0").matches(Version("1.0.0+foo"), VersionMatchMode.strict)); + logDebug("Dependency unittest success."); } diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index c0a95333e..fdcfa4b3f 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -316,9 +316,11 @@ class PackageManager { Package getBestPackage(string name, Dependency version_spec, bool enable_overrides = true) { Package ret; - foreach (p; getPackageIterator(name)) - if (version_spec.matches(p.version_) && (!ret || p.version_ > ret.version_)) + foreach (p; getPackageIterator(name)) { + auto vmm = isManagedPackage(p) ? VersionMatchMode.strict : VersionMatchMode.standard; + if (version_spec.matches(p.version_, vmm) && (!ret || p.version_ > ret.version_)) ret = p; + } if (enable_overrides && ret) { if (auto ovr = getPackage(name, ret.version_)) From fe0894cc62ff2c073caf1b5b2b02e8ef684afc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Wed, 6 Jul 2022 11:56:42 +0200 Subject: [PATCH 4/5] Add test for #2262. --- test/issue2262-exact-cached-version-match.sh | 49 +++++++++++++++++++ .../.no_build | 0 .../dub.sdl | 2 + .../source/app.d | 1 + 4 files changed, 52 insertions(+) create mode 100755 test/issue2262-exact-cached-version-match.sh create mode 100644 test/issue2262-exact-cached-version-match/.no_build create mode 100644 test/issue2262-exact-cached-version-match/dub.sdl create mode 100644 test/issue2262-exact-cached-version-match/source/app.d diff --git a/test/issue2262-exact-cached-version-match.sh b/test/issue2262-exact-cached-version-match.sh new file mode 100755 index 000000000..faa5e5b76 --- /dev/null +++ b/test/issue2262-exact-cached-version-match.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash + +. $(dirname ${BASH_SOURCE[0]})/common.sh + +PACK_PATH="$CURR_DIR"/issue2262-exact-cached-version-match + +# make sure that there are no left-over selections files +rm -f $PACK_PATH/dub.selections.json + +# make sure that there are no cached versions of the dependency +dub remove gitcompatibledubpackage@* -n || true + +# build normally, should select 1.0.4 +if ! ${DUB} build --root $PACK_PATH | grep "gitcompatibledubpackage 1\.0\.4:"; then + die $LINENO 'The initial build failed.' +fi +dub remove gitcompatibledubpackage@* -n || true + +# build with git dependency to a specific commit +cat > $PACK_PATH/dub.selections.json << EOF +{ + "fileVersion": 1, + "versions": { + "gitcompatibledubpackage": { + "repository": "git+https://github.com/dlang-community/gitcompatibledubpackage.git", + "version": "ccb31bf6a655437176ec02e04c2305a8c7c90d67" + } + } +} +EOF +if ! ${DUB} build --root $PACK_PATH | grep "gitcompatibledubpackage 1\.0\.4+commit\.2\.gccb31bf:"; then + die $LINENO 'The build with a specific commit failed.' +fi + +# select 1.0.4 again +cat > $PACK_PATH/dub.selections.json << EOF +{ + "fileVersion": 1, + "versions": { + "gitcompatibledubpackage": "1.0.4" + } +} +EOF +if ! ${DUB} build --root $PACK_PATH | grep "gitcompatibledubpackage 1\.0\.4:"; then + die $LINENO 'The second 1.0.4 build failed.' +fi + +# clean up +rm -f $PACK_PATH/dub.selections.json diff --git a/test/issue2262-exact-cached-version-match/.no_build b/test/issue2262-exact-cached-version-match/.no_build new file mode 100644 index 000000000..e69de29bb diff --git a/test/issue2262-exact-cached-version-match/dub.sdl b/test/issue2262-exact-cached-version-match/dub.sdl new file mode 100644 index 000000000..90f9893f9 --- /dev/null +++ b/test/issue2262-exact-cached-version-match/dub.sdl @@ -0,0 +1,2 @@ +name "testproj" +dependency "gitcompatibledubpackage" version="~>1.0.4" diff --git a/test/issue2262-exact-cached-version-match/source/app.d b/test/issue2262-exact-cached-version-match/source/app.d new file mode 100644 index 000000000..ab73b3a23 --- /dev/null +++ b/test/issue2262-exact-cached-version-match/source/app.d @@ -0,0 +1 @@ +void main() {} From 5d69aaeafe6ec9079d614b56a00ddf9b6f1a98e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Wed, 6 Jul 2022 12:26:37 +0200 Subject: [PATCH 5/5] Also use strict version matching for cached packages when determining the need to fetch a dependency. Fixes #2262. --- source/dub/dub.d | 5 +++-- source/dub/packagemanager.d | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/source/dub/dub.d b/source/dub/dub.d index 0bc56eb2e..65840c9e7 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -532,7 +532,7 @@ class Dub { foreach (ps; m_packageSuppliers) { try { auto versions = ps.getVersions(p); - if (versions.canFind!(v => dep.matches(v))) + if (versions.canFind!(v => dep.matches(v, VersionMatchMode.strict))) continue next_pack; } catch (Exception e) { logWarn("Error querying versions for %s, %s: %s", p, ps.description, e.msg); @@ -592,7 +592,8 @@ class Dub { } else if (!ver.repository.empty) { pack = m_packageManager.loadSCMPackage(p, ver); } else { - pack = m_packageManager.getBestPackage(p, ver); + assert(ver.isExactVersion, "Resolved dependency is neither path, nor repository, nor exact version based!?"); + pack = m_packageManager.getPackage(p, ver.version_); if (pack && m_packageManager.isManagedPackage(pack) && ver.version_.isBranch && (options & UpgradeOptions.upgrade) != 0) { diff --git a/source/dub/packagemanager.d b/source/dub/packagemanager.d index fdcfa4b3f..af16b89fa 100644 --- a/source/dub/packagemanager.d +++ b/source/dub/packagemanager.d @@ -188,9 +188,11 @@ class PackageManager { /// ditto Package getPackage(string name, Version ver, NativePath path) { - foreach (p; getPackageIterator(name)) - if (p.version_ == ver && p.path.startsWith(path)) + foreach (p; getPackageIterator(name)) { + auto pvm = isManagedPackage(p) ? VersionMatchMode.strict : VersionMatchMode.standard; + if (p.version_.matches(ver, pvm) && p.path.startsWith(path)) return p; + } return null; }