Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support dependencies as git url with exact commit #1765

Merged
merged 1 commit into from
Oct 8, 2019
Merged

support dependencies as git url with exact commit #1765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 98 additions & 6 deletions source/dub/dependency.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ struct PackageDependency {
Dependency spec;
}


/**
Represents a dependency specification.

Expand All @@ -56,6 +55,7 @@ struct Dependency {
NativePath m_path;
bool m_optional = false;
bool m_default = false;
Repository m_repository;
}

/// A Dependency, which matches every valid version.
Expand Down Expand Up @@ -93,11 +93,28 @@ struct Dependency {
m_path = path;
}

this(Repository repository, string spec) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docs for this constructor overload.

this.versionSpec = spec;
this.repository = repository;
}

/// If set, overrides any version based dependency selection.
@property void path(NativePath value) { m_path = value; }
/// ditto
@property NativePath path() const { return m_path; }

/// If set, overrides any version based dependency selection.
@property void repository(Repository value)
{
m_repository = value;
}

/// ditto
@property Repository repository() const
{
return m_repository;
}

/// Determines if the dependency is required or optional.
@property bool optional() const { return m_optional; }
/// ditto
Expand All @@ -111,6 +128,8 @@ struct Dependency {
/// Returns true $(I iff) the version range only matches a specific version.
@property bool isExactVersion() const { return m_versA == m_versB; }

@property bool isGit() const { return !repository.empty; }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docs on this new property.


/// Returns the exact version matched by the version range.
@property Version version_() const {
enforce(m_versA == m_versB, "Dependency "~this.versionSpec~" is no exact version.");
Expand Down Expand Up @@ -156,7 +175,7 @@ struct Dependency {
ves = ves[2..$];
m_versA = Version(expandVersion(ves));
m_versB = Version(bumpVersion(ves) ~ "-0");
} else if (ves[0] == Version.branchPrefix) {
} else if (ves[0] == Version.branchPrefix || ves.isHash) {
m_inclusiveA = true;
m_inclusiveB = true;
m_versA = m_versB = Version(ves);
Expand Down Expand Up @@ -207,7 +226,6 @@ struct Dependency {
string r;

if (this == invalid) return "invalid";

if (m_versA == m_versB && m_inclusiveA && m_inclusiveB) {
// Special "==" case
if (m_versA == Version.masterBranch) return "~master";
Expand Down Expand Up @@ -258,7 +276,12 @@ struct Dependency {
*/
string toString()()
const {
auto ret = versionSpec;
string ret;

if (!repository.empty) {
ret ~= "#";
}
ret ~= versionSpec;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an expert on all the options here, but this formulation looks questionable: is it legit for the git version spec to be followed by extra stuff (which will happen if optional is true, or if !path.empty)?

Also, why the # symbol?

if (optional) {
if (default_) ret ~= " (optional, default)";
else ret ~= " (optional)";
Expand Down Expand Up @@ -288,6 +311,7 @@ struct Dependency {
json = Json.emptyObject;
json["version"] = this.versionSpec;
if (!path.empty) json["path"] = path.toString();
if (!repository.empty) json["repository"] = repository.remote;
if (optional) json["optional"] = true;
if (default_) json["default"] = true;
}
Expand Down Expand Up @@ -316,6 +340,10 @@ struct Dependency {

dep = Dependency.any;
dep.path = NativePath(verspec["path"].get!string);
} else if (auto repository = "repository" in verspec) {
enforce("version" in verspec, "No version field specified!");
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;
Expand Down Expand Up @@ -397,6 +425,7 @@ struct Dependency {
A specification is valid if it can match at least one version.
*/
bool valid() const {
if (isGit) return true;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest being consistent in usage: this.isGit (as elsewhere), not just isGit

return m_versA <= m_versB && doCmp(m_inclusiveA && m_inclusiveB, m_versA, m_versB);
}

Expand Down Expand Up @@ -426,6 +455,7 @@ struct Dependency {
/// ditto
bool matches(ref const(Version) v) const {
if (this.matchesAny) return true;
if (this.isGit) return true;
//logDebug(" try match: %s with: %s", v, this);
// Master only matches master
if(m_versA.isBranch) {
Expand All @@ -449,6 +479,13 @@ struct Dependency {
*/
Dependency merge(ref const(Dependency) o)
const {
if (this.isGit) {
if (!o.isGit) return this;
if (this.m_versA == o.m_versB) return this;
return invalid;
}
if (o.isGit) return o;

if (this.matchesAny) return o;
if (o.matchesAny) return this;
if (m_versA.isBranch != o.m_versA.isBranch) return invalid;
Expand Down Expand Up @@ -653,6 +690,34 @@ unittest {
assert(Dependency("~>1.0.4+1.2.3").versionSpec == "~>1.0.4");
}

/**
Represents an SCM repository.
*/
struct Repository
{
private string m_remote;

/**
Returns:
Repository URL or path.
*/
@property string remote() @nogc nothrow pure @safe
in { assert(m_remote !is null); }
body
{
return m_remote;
}

/**
Returns:
Whether the repository was initialized with an URL or path.
*/
@property bool empty() const @nogc nothrow pure @safe
{
return m_remote is null;
}
}


/**
Represents a version in semantic version format, or a branch identifier.
Expand Down Expand Up @@ -681,7 +746,7 @@ struct Version {
this(string vers)
{
enforce(vers.length > 1, "Version strings must not be empty.");
if (vers[0] != branchPrefix && vers.ptr !is UNKNOWN_VERS.ptr)
if (vers[0] != branchPrefix && !vers.isHash && vers.ptr !is UNKNOWN_VERS.ptr)
enforce(vers.isValidVersion(), "Invalid SemVer format: " ~ vers);
m_version = vers;
}
Expand All @@ -695,6 +760,9 @@ struct Version {

bool opEquals(const Version oth) const { return opCmp(oth) == 0; }

/// Tests if this represents a hash instead of a version.
@property bool isGit() const { return m_version.isHash; }

/// Tests if this represents a branch instead of a version.
@property bool isBranch() const { return m_version.length > 0 && m_version[0] == branchPrefix; }

Expand All @@ -706,7 +774,7 @@ struct Version {
Note that branches are always considered pre-release versions.
*/
@property bool isPreRelease() const {
if (isBranch) return true;
if (isBranch || isGit) return true;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should isGit necessarily imply a pre-release? What if the git version spec points to a commit with a release tag?

return isPreReleaseVersion(m_version);
}

Expand All @@ -725,6 +793,13 @@ struct Version {
if (isUnknown || other.isUnknown) {
throw new Exception("Can't compare unknown versions! (this: %s, other: %s)".format(this, other));
}

if (isGit || other.isGit) {
if (!isGit) return -1;
if (!other.isGit) return 1;
return (m_version == m_version) ? 0 : 1;
}

if (isBranch || other.isBranch) {
if(m_version == other.m_version) return 0;
if (!isBranch) return 1;
Expand Down Expand Up @@ -766,6 +841,7 @@ unittest {
assert(a == b, "a == b with a:'1.0.0', b:'1.0.0' failed");
b = Version("2.0.0");
assert(a != b, "a != b with a:'1.0.0', b:'2.0.0' failed");

a = Version.masterBranch;
b = Version("~BRANCH");
assert(a != b, "a != b with a:MASTER, b:'~branch' failed");
Expand Down Expand Up @@ -805,4 +881,20 @@ unittest {
assertThrown(a == b, "Failed: UNKNOWN == UNKNOWN");

assert(Version("1.0.0+a") == Version("1.0.0+b"));

assert(Version("73535568b79a0b124bc1653002637a830ce0fcb8").isGit);
}

private bool isHash(string hash) @nogc nothrow pure @safe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing function documentation

{
import std.ascii : isAlphaNum;
import std.utf : byCodeUnit;

return hash.length == 40 && hash.byCodeUnit.all!isAlphaNum;
}

@nogc nothrow pure @safe unittest {
assert(isHash("73535568b79a0b124bc1653002637a830ce0fcb8"));
assert(!isHash("735"));
assert(!isHash("73535568b79a0b124bc1-53002637a830ce0fcb8"));
}
20 changes: 16 additions & 4 deletions source/dub/dub.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,9 @@ class Dub {
if (!path.absolute) path = this.rootPath ~ path;
try if (m_packageManager.getOrLoadPackage(path)) continue;
catch (Exception e) { logDebug("Failed to load path based selection: %s", e.toString().sanitize); }
} else if (!dep.repository.empty) {
if (m_packageManager.clonePackage(getBasePackageName(p), dep))
continue;
} else {
if (m_packageManager.getPackage(p, dep.version_)) continue;
foreach (ps; m_packageSuppliers) {
Expand Down Expand Up @@ -534,7 +537,7 @@ class Dub {
string rootbasename = getBasePackageName(m_project.rootPackage.name);

foreach (p, ver; versions) {
if (!ver.path.empty) continue;
if (!ver.path.empty || !ver.repository.empty) continue;

auto basename = getBasePackageName(p);
if (basename == rootbasename) continue;
Expand All @@ -546,7 +549,7 @@ class Dub {
continue;
}
auto sver = m_project.selections.getSelectedVersion(basename);
if (!sver.path.empty) continue;
if (!sver.path.empty || !sver.repository.empty) continue;
if (ver.version_ <= sver.version_) continue;
logInfo("Package %s would be upgraded from %s to %s.",
basename, sver, ver);
Expand All @@ -566,6 +569,8 @@ class Dub {
logDebug("Failed to load path based selection: %s", e.toString().sanitize);
continue;
}
} else if (!ver.repository.empty) {
pack = m_packageManager.clonePackage(p, ver);
} else {
pack = m_packageManager.getBestPackage(p, ver);
if (pack && m_packageManager.isManagedPackage(pack)
Expand All @@ -582,7 +587,9 @@ class Dub {
fetchOpts |= (options & UpgradeOptions.preRelease) != 0 ? FetchOptions.usePrerelease : FetchOptions.none;
if (!pack) fetch(p, ver, defaultPlacementLocation, fetchOpts, "getting selected version");
if ((options & UpgradeOptions.select) && p != m_project.rootPackage.name) {
if (ver.path.empty) m_project.selections.selectVersion(p, ver.version_);
if (!ver.repository.empty) {
m_project.selections.selectVersion(p, ver.repository, ver.versionSpec);
} else if (ver.path.empty) m_project.selections.selectVersion(p, ver.version_);
else {
NativePath relpath = ver.path;
if (relpath.absolute) relpath = relpath.relativeTo(m_project.rootPackage.path);
Expand Down Expand Up @@ -1533,6 +1540,8 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
protected override Dependency[] getSpecificConfigs(string pack, TreeNodes nodes)
{
if (!nodes.configs.path.empty && getPackage(pack, nodes.configs)) return [nodes.configs];
else if (!nodes.configs.repository.empty && getPackage(pack, nodes.configs))
return [nodes.configs];
else return null;
}

Expand Down Expand Up @@ -1652,7 +1661,10 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
if (basename == m_rootPackage.basePackage.name)
return m_rootPackage.basePackage;

if (!dep.path.empty) {
if (!dep.repository.empty) {
auto ret = m_dub.packageManager.clonePackage(name, dep);
return ret !is null && dep.matches(ret.version_) ? ret : null;
} else if (!dep.path.empty) {
try {
auto ret = m_dub.packageManager.getOrLoadPackage(dep.path);
if (dep.matches(ret.version_)) return ret;
Expand Down
49 changes: 49 additions & 0 deletions source/dub/packagemanager.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,55 @@ class PackageManager {
return pack;
}

/** For a given Git repository, returns the corresponding package.

Git repository is provided as its remote URL, the repository is cloned
and in the dependency speicfied commit is checked out.

If the target directory already exists, just returns the package
without cloning.

Params:
name = Package name
dependency = Dependency that contains the repository URL and a specific commit

Returns:
The package loaded from the given Git repository or null if the
package couldn't be loaded.
*/
Package clonePackage(string name, Dependency dependency)
in { assert(!dependency.repository.empty); }
body {
import std.process : escapeShellCommand, executeShell;

string gitReference = dependency.versionSpec;
if (gitReference.startsWith("~")) gitReference.popFront;

const destination = m_repositories[LocalPackageType.user].packagePath ~
NativePath(name~"-"~gitReference) ~ name;
const nativeDestination = destination.toNativeString();

if (!exists(nativeDestination)) {
auto command = escapeShellCommand("git", "clone",
dependency.repository.remote,
nativeDestination);
if (executeShell(command).status != 0) {
return null;
}

command = escapeShellCommand("git",
"-C", nativeDestination,
"checkout", gitReference);
if (executeShell(command).status != 0) {
rmdirRecurse(nativeDestination);
return null;
}
}
auto pack = Package.load(destination);
addPackages(m_temporaryPackages, pack);

return pack;
}

/** Searches for the latest version of a package matching the given dependency.
*/
Expand Down
Loading