From 663d9d2a83cf80722d53b3c73a9463a63e9942cf Mon Sep 17 00:00:00 2001 From: Geod24 Date: Wed, 13 Jul 2022 13:46:03 +0200 Subject: [PATCH] Dependency: Move `spec` / git ref inside of `Repository` The `Dependency` struct is logically acting as an `enum`, with 3 discrete possibilities. However, some of the logic is piggybacking on the (original) version logic. `Repository` is one of them, and by separating things, we can make stronger assumptions in `Version` and `Dependency`. --- source/dub/dependency.d | 65 ++++++++++++++++++++++++++++++++--------- source/dub/dub.d | 2 +- source/dub/project.d | 14 ++++++--- source/dub/recipe/sdl.d | 8 ++--- 4 files changed, 66 insertions(+), 23 deletions(-) diff --git a/source/dub/dependency.d b/source/dub/dependency.d index 301e339dc..ddf1ca794 100644 --- a/source/dub/dependency.d +++ b/source/dub/dependency.d @@ -98,9 +98,20 @@ struct Dependency { /** Constructs a new dependency specification that matches a specific Git reference. */ - this(Repository repository, string spec) { - this.versionSpec = spec; + this(Repository repository) + { this.repository = repository; + // Set for backward compatibility + auto ver = Version(repository.m_ref); + this.m_range = VersionRange(ver, ver, true, true); + } + + deprecated("Instantiate the `Repository` struct with the string directy") + this(Repository repository, string spec) + { + assert(repository.m_ref is null); + repository.m_ref = spec; + this(repository); } /// If set, overrides any version based dependency selection. @@ -112,6 +123,9 @@ struct Dependency { @property void repository(Repository value) { m_repository = value; + // Set for backward compatibility + auto ver = Version(value.m_ref); + this.m_range = VersionRange(ver, ver, true, true); } /// ditto @@ -246,7 +260,7 @@ struct Dependency { } @trusted unittest { - Dependency dependency = Dependency(Repository("git+http://localhost"), "1.0.0"); + Dependency dependency = Dependency(Repository("git+http://localhost", "1.0.0")); Json expected = Json([ "repository": Json("git+http://localhost"), "version": Json("1.0.0") @@ -272,8 +286,8 @@ struct Dependency { enforce("version" in verspec, "No version field specified!"); enforce(repository.length > 0, "No repository field specified!"); - dep = Dependency(Repository(repository.get!string), - verspec["version"].get!string); + dep = Dependency(Repository( + repository.get!string, verspec["version"].get!string)); } else { enforce("version" in verspec, "No version field specified!"); auto ver = verspec["version"].get!string; @@ -606,6 +620,7 @@ unittest { struct Repository { private string m_remote; + private string m_ref; private Kind m_kind; @@ -617,18 +632,28 @@ struct Repository /** Params: remote = Repository remote. + ref_ = Reference to use (SHA1, tag, branch name...) */ + this(string remote, string ref_) + { + enforce(remote.startsWith("git+"), "Unsupported repository type"); + + m_remote = remote["git+".length .. $]; + m_kind = Kind.git; + m_ref = ref_; + assert(m_remote.length); + assert(m_ref.length); + } + + /// Ditto + deprecated("Use the constructor accepting a second parameter named `ref_`") this(string remote) { - if (remote.startsWith("git+")) - { - m_remote = remote["git+".length .. $]; - m_kind = Kind.git; - } - else - { - throw new Exception("Unsupported repository type"); - } + enforce(remote.startsWith("git+"), "Unsupported repository type"); + + m_remote = remote["git+".length .. $]; + m_kind = Kind.git; + assert(m_remote.length); } string toString() nothrow pure @safe @@ -655,6 +680,18 @@ struct Repository return m_remote; } + /** + Returns: + The reference (commit hash, branch name, tag) we are targeting + */ + @property string ref_() @nogc nothrow pure @safe + in { assert(m_remote !is null); } + in { assert(m_ref !is null); } + do + { + return m_ref; + } + /** Returns: Repository type. diff --git a/source/dub/dub.d b/source/dub/dub.d index 76636c6cf..7ea72687a 100644 --- a/source/dub/dub.d +++ b/source/dub/dub.d @@ -612,7 +612,7 @@ class Dub { if (!pack) fetch(p, ver, defaultPlacementLocation, fetchOpts, "getting selected version"); if ((options & UpgradeOptions.select) && p != m_project.rootPackage.name) { if (!ver.repository.empty) { - m_project.selections.selectVersionWithRepository(p, ver.repository, ver.versionSpec); + m_project.selections.selectVersionWithRepository(p, ver.repository); } else if (ver.path.empty) { m_project.selections.selectVersion(p, ver.version_); } else { diff --git a/source/dub/project.d b/source/dub/project.d index 226442080..0a26b8c0e 100644 --- a/source/dub/project.d +++ b/source/dub/project.d @@ -1764,9 +1764,9 @@ final class SelectedVersions { } /// Selects a certain Git reference for a specific package. - void selectVersionWithRepository(string package_id, Repository repository, string spec) + void selectVersionWithRepository(string package_id, Repository repository) { - const dependency = Dependency(repository, spec); + const dependency = Dependency(repository); if (auto ps = package_id in m_selections) { if (ps.dep == dependency) return; @@ -1775,6 +1775,12 @@ final class SelectedVersions { m_dirty = true; } + deprecated("Move `spec` inside of the `repository` parameter") + void selectVersionWithRepository(string package_id, Repository repository, string spec) + { + this.selectVersionWithRepository(package_id, Repository(repository.remote(), spec)); + } + /// Removes the selection for a particular package. void deselectVersion(string package_id) { @@ -1853,8 +1859,8 @@ final class SelectedVersions { else if (j.type == Json.Type.object && "path" in j) return Dependency(NativePath(j["path"].get!string)); else if (j.type == Json.Type.object && "repository" in j) - return Dependency(Repository(j["repository"].get!string), - enforce("version" in j, "Expected \"version\" field in repository version object").get!string); + return Dependency(Repository(j["repository"].get!string, + enforce("version" in j, "Expected \"version\" field in repository version object").get!string)); else throw new Exception(format("Unexpected type for dependency: %s", j)); } diff --git a/source/dub/recipe/sdl.d b/source/dub/recipe/sdl.d index 089756227..8b4038b56 100644 --- a/source/dub/recipe/sdl.d +++ b/source/dub/recipe/sdl.d @@ -194,8 +194,8 @@ private void parseDependency(Tag t, ref BuildSettingsTemplate bs, string package } else if ("repository" in attrs) { enforceSDL("version" in attrs, "Missing version specification.", t); - dep.repository = Repository(attrs["repository"][0].value.get!string); - dep.versionSpec = attrs["version"][0].value.get!string; + dep.repository = Repository(attrs["repository"][0].value.get!string, + attrs["version"][0].value.get!string); } else { enforceSDL("version" in attrs, "Missing version specification.", t); dep.versionSpec = attrs["version"][0].value.get!string; @@ -672,8 +672,8 @@ unittest { PackageRecipe p; p.name = "test"; - auto repository = Repository("git+https://some.url"); - p.buildSettings.dependencies["package"] = Dependency(repository, "12345678"); + auto repository = Repository("git+https://some.url", "12345678"); + p.buildSettings.dependencies["package"] = Dependency(repository); auto sdl = toSDL(p).toSDLDocument(); assert(sdl == `name "test"