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

[WIP] support dependencies as git url (including with exact commit hash) #1403

Closed
wants to merge 1 commit into from
Closed

[WIP] support dependencies as git url (including with exact commit hash) #1403

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
115 changes: 111 additions & 4 deletions source/dub/dependency.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,62 @@ struct PackageDependency {
Dependency spec;
}

struct GitDep{
Copy link
Member

Choose a reason for hiding this comment

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

Style: Should be fully spelled out (GitDependency) and before { there should always be a single space (or a line break in case of functions).

private{
string urlPackage;
string commit;
string spec;
}

bool valid()const @safe{
return !!spec;
}

string toURL(){
return format(`%s/commit/%s`, urlPackage, commit);
}

URL toURLZip(){
URL ret=format(`%s/archive/%s.zip`, urlPackage, commit);
return ret;
}

string toVersion(){
auto ret=spec;
// IMPROVE
ret=ret.replace(`/`, `#`);
ret=ret.replace(`:`, `$`);
return ret;
}

bool isSame(ref const GitDep other)const @safe{
return this.commit==other.commit;
}

void from(string spec){
spec=spec.replace(`#`, `/`);
spec=spec.replace(`$`, `:`);
this.spec=spec;
/+
EG:
@https://github.com/CyberShadow/ae/commit/abc177c494c0ed75bc1671fbd4ebdd8323a4d734
+/
assert(spec[0]==Version.hashPrefix);
import std.regex;
auto m=spec[1..$].matchFirst(`^(.*?)/commit/([a-f0-9]+)$`.regex);
Copy link
Member

Choose a reason for hiding this comment

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

this will compile the regex on every execution. While std.regex now comes with a bit of caching, it's still better to do sth. like

static re = `...`.regex;
spec[1..$].matchFirst(re);

assert(!m.empty, spec);
urlPackage=m.captures[1];
commit=m.captures[2];
}

this(string spec)@trusted{
from(spec);
}

string toString()const @safe{
return spec;
}
}

/**
Represents a dependency specification.
Expand Down Expand Up @@ -79,6 +135,10 @@ struct Dependency {
*/
this(in Version ver)
{
// PRTEMP
if(ver.m_version[0]==Version.hashPrefix){
this.versionSpec = ver.m_version;
}
m_inclusiveA = m_inclusiveB = true;
m_versA = ver;
m_versB = ver;
Expand All @@ -98,6 +158,8 @@ struct Dependency {
/// ditto
@property NativePath path() const { return m_path; }

ref inout(GitDep) gitDep() inout { assert(isGit); return m_versA.gitDep; }

/// Determines if the dependency is required or optional.
@property bool optional() const { return m_optional; }
/// ditto
Expand All @@ -111,6 +173,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 m_versA.isGit; }

/// 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 @@ -154,7 +218,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[0] == Version.hashPrefix) {
m_inclusiveA = true;
m_inclusiveB = true;
m_versA = m_versB = Version(ves);
Expand Down Expand Up @@ -203,7 +267,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 @@ -365,6 +428,9 @@ struct Dependency {
/// ditto
int opCmp(in Dependency o)
const {
// PRTEMP
//if (this.isGit || o.isGit) return m_versA.opCmp(o.m_versA);

if (m_inclusiveA != o.m_inclusiveA) return m_inclusiveA < o.m_inclusiveA ? -1 : 1;
if (m_inclusiveB != o.m_inclusiveB) return m_inclusiveB < o.m_inclusiveB ? -1 : 1;
if (m_versA != o.m_versA) return m_versA < o.m_versA ? -1 : 1;
Expand Down Expand Up @@ -393,6 +459,7 @@ struct Dependency {
A specification is valid if it can match at least one version.
*/
bool valid() const {
if(isGit) return true;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be generalized to isHash?

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

Expand Down Expand Up @@ -445,6 +512,16 @@ struct Dependency {
*/
Dependency merge(ref const(Dependency) o)
const {
// PRTEMP
if(this.isGit || o.isGit){
if(!o.isGit) return this;
if(!this.isGit) return o;
if(this.gitDep.isSame(o.gitDep)) return this;
// CHECKME
return this;
//return invalid;
}

if (this.matchesAny) return o;
if (o.matchesAny) return this;
if (m_versA.isBranch != o.m_versA.isBranch) return invalid;
Expand All @@ -470,7 +547,8 @@ struct Dependency {
private static bool isDigit(char ch) { return ch >= '0' && ch <= '9'; }
private static string skipComp(ref string c) {
size_t idx = 0;
while (idx < c.length && !isDigit(c[idx]) && c[idx] != Version.branchPrefix) idx++;
// PRTEMP
while (idx < c.length && !isDigit(c[idx]) && c[idx] != Version.branchPrefix && c[idx] != Version.hashPrefix) idx++;
enforce(idx < c.length, "Expected version number in version spec: "~c);
string cmp = idx==c.length-1||idx==0? ">=" : c[0..idx];
c = c[idx..$];
Expand Down Expand Up @@ -666,6 +744,8 @@ struct Version {
enum branchPrefix = '~';
string m_version;
}
enum hashPrefix = '@'; // RENAME
GitDep gitDep;

static immutable Version minRelease = Version("0.0.0");
static immutable Version maxRelease = Version(MAX_VERS);
Expand All @@ -677,9 +757,12 @@ 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[0] != hashPrefix && vers.ptr !is UNKNOWN_VERS.ptr)
enforce(vers.isValidVersion(), "Invalid SemVer format: " ~ vers);
m_version = vers;
if(vers[0] == hashPrefix){
gitDep=GitDep(vers);
}
}

/** Constructs a new `Version` from its string representation.
Expand All @@ -689,8 +772,12 @@ struct Version {
*/
static Version fromString(string vers) { return Version(vers); }

// TODO: for git, could throw but shouldn't in this context
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 gitDep.valid; }

/// 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 @@ -703,6 +790,7 @@ struct Version {
*/
@property bool isPreRelease() const {
if (isBranch) return true;
if (isGit) return true;
return isPreReleaseVersion(m_version);
}

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

// PRTEMP
//// isGit has precedence
if (isGit || other.isGit) {
if(!isGit) return -1;
if(!other.isGit) return 1;
//if(m_version == other.m_version) return 0;
if(gitDep.isSame(other.gitDep)) return 0;


// CHECKME: we want to pick what's asked for; is there a better way?
return 1;
//throw new Exception("Can't compare versions! (this: %s, other: %s)".format(this.gitDep, other.gitDep));
}

if (isBranch || other.isBranch) {
if(m_version == other.m_version) return 0;
if (!isBranch) return 1;
Expand Down Expand Up @@ -762,6 +865,10 @@ 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");

// PRTEMP
assert(a<b);

a = Version.masterBranch;
b = Version("~BRANCH");
assert(a != b, "a != b with a:MASTER, b:'~branch' failed");
Expand Down
12 changes: 11 additions & 1 deletion source/dub/dub.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ class Dub {

init();

PackageSupplier[] ps = additional_package_suppliers;
PackageSupplier[] ps;
ps ~= new GitPackageSupplier();
ps ~= additional_package_suppliers;

if (skip_registry < SkipPackageSuppliers.all)
{
Expand Down Expand Up @@ -426,6 +428,8 @@ class Dub {
next_pack:
foreach (p; m_project.selections.selectedPackages) {
auto dep = m_project.selections.getSelectedVersion(p);
if(dep.isGit)
continue next_pack;
if (!dep.path.empty) {
auto path = dep.path;
if (!path.absolute) path = this.rootPath ~ path;
Expand All @@ -446,6 +450,8 @@ class Dub {
}

logWarn("Selected package %s %s doesn't exist. Using latest matching version instead.", p, dep);
// CHECKME: cleaner to error in this case no? unless maybe the idea is if user has no control over that because of third party error?
//assert(0);
m_project.selections.deselectVersion(p);
}
}
Expand Down Expand Up @@ -1536,9 +1542,13 @@ private class DependencyVersionResolver : DependencyResolver!(Dependency, Depend
if (basename == m_rootPackage.basePackage.name)
return m_rootPackage.basePackage;

if (dep.isGit)
return m_dub.fetch(basename, dep, m_dub.defaultPlacementLocation, FetchOptions.init, "need commit hash");

if (!dep.path.empty) {
try {
auto ret = m_dub.packageManager.getOrLoadPackage(dep.path);
// TODO: shouldn't that allow version + path?
if (dep.matches(ret.version_)) return ret;
} catch (Exception e) {
logDiagnostic("Failed to load path based dependency %s: %s", name, e.msg);
Expand Down
44 changes: 44 additions & 0 deletions source/dub/packagesupplier.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,50 @@ class FileSystemPackageSupplier : PackageSupplier {
}
}

/**
Git based based package supplier.

This package supplier downloads directly from git.
*/
class GitPackageSupplier : PackageSupplier {
this() { }

override @property string description() { return typeof(this).stringof; }

Version[] getVersions(string package_id)
{
return null;
}

void fetchPackage(NativePath path, string packageId, Dependency dep, bool pre_release)
{
enforce(path.absolute);
logInfo("Storing package '"~packageId~"', version requirements: %s", dep);
auto gitDep=dep.gitDep;
assert(gitDep.valid);
URL url = gitDep.toURLZip;
logDiagnostic("Downloading from '%s'", url);
// TODO: foreach(i; 0..3)
download(url, path);
Copy link
Member

Choose a reason for hiding this comment

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

Might be worthwhile to generalize the retry logic in download (almost all supplier seem to require this)

}

Json fetchPackageRecipe(string packageId, Dependency dep, bool pre_release)
{
if(!dep.isGit)
return Json(null);
auto gitDep=dep.gitDep;
auto ret=Json.emptyObject;
ret["name"]=packageId;
ret["version"]=gitDep.toVersion;
return ret;
}

SearchResult[] searchPackages(string query)
{
assert(0);
}
}


/**
Online registry based package supplier.
Expand Down