quarks (new) - dependencies specified with git url and refspec not working ? #1377

Closed
miguel-negrao opened this Issue Mar 27, 2015 · 4 comments

Comments

Projects
None yet
3 participants
@miguel-negrao
Member

miguel-negrao commented Mar 27, 2015

the help says

dependencies is a list of Quarks or git urls with optional an @refspec

but using a git url with a refspec in the dependencies list (url@refspec) doesn't seem to work. I think the issue is that Quarks.new does not check for git urls like url@refspec like Quarks.load does:

    *parseQuarkName { |name, refspec|
        // determine which quark the string 'name' refers to
        // and return name, url, refspec, localPath
        // name is one of: quarkname, url, localPath
        var directoryEntry;
        if(name.contains("://"), {
            ^[PathName(name).fileNameWithoutExtension(), name, refspec, nil]
        });

here refspec is not extracted from name, if the name is of type url@refspec.

Btw, the new Quarks system rock !!!!

@miguel-negrao miguel-negrao changed the title from quarks (new) - dependencies specified with git url not working ? to quarks (new) - dependencies specified with git url and refspec not working ? Mar 28, 2015

@miguel-negrao miguel-negrao added the bug label Mar 28, 2015

@scztt scztt added this to the 3.7 milestone Apr 12, 2015

scztt referenced this issue Apr 13, 2015

classlib: Parse @refspec pattern in quark urls
Quarks specified using a "thequark@1.2.3" pattern should be parsed as having a refspec of "1.2.3". If a refspec is ALSO provided via an argument, and it conflicts with the refspec in the url, we error.
@scztt

This comment has been minimized.

Show comment
Hide comment
@scztt

scztt Apr 13, 2015

Contributor

After looking at this in a little more detail - I wonder if the documentation should simply be changed? Supporting dependencies of the form "quark@refspec" AND "quark" -> "refspec" doesn't seem necessary, and opens the door to needless ambiguity (i.e. "quark@1.1" -> "1.2"). The association form does everything we need just fine.

Contributor

scztt commented Apr 13, 2015

After looking at this in a little more detail - I wonder if the documentation should simply be changed? Supporting dependencies of the form "quark@refspec" AND "quark" -> "refspec" doesn't seem necessary, and opens the door to needless ambiguity (i.e. "quark@1.1" -> "1.2"). The association form does everything we need just fine.

@miguel-negrao

This comment has been minimized.

Show comment
Hide comment
@miguel-negrao

miguel-negrao Apr 13, 2015

Member

You mean

dependencies: ["Bjorkland", "Canvas3D", "cruciallib"->"tags/4.1.4"]

is currently supported ? that's not stated in the "Using Quarks" helps file...

Member

miguel-negrao commented Apr 13, 2015

You mean

dependencies: ["Bjorkland", "Canvas3D", "cruciallib"->"tags/4.1.4"]

is currently supported ? that's not stated in the "Using Quarks" helps file...

@crucialfelix

This comment has been minimized.

Show comment
Hide comment
@crucialfelix

crucialfelix Apr 13, 2015

Member

The only reason it supports quark -> version is for backward compatibility.
Because all of the current quarks have their dependencies written that way.

But it can only be written that way in supercollider code. Its an
Association, not a formal syntax.

So I figured the directory.txt should not use this arrow thingy, it should
use something in common usage to other package managements systems.
which is @

And its natural to expect that if directory.txt allows @ then dependencies
should also allow it.

Why have two separate ways of specifying a quark and its version ?

But I didn't even implement support for @ in the dependencies and didn't
realize that I didn't until Miguel filed this bug.
IMO the dependencies list should support @ and that should be the
documented and recommended way to do it.

-> is for backwards compatibility

until somebody actually decides to write both @ and -> then we shouldn't
worry about it. who would do such a thing ?

So I will finish implementing @ in dependencies (and I found another bug in
that method)

and I will improve documentation to say that @ is the way to express it

and that -> is there for backwards compatibility

The bigger issue is that old quarks do not have tags, so we cannot check
them out by versions.
We could automatically tag them, but I think this is overkill.

We could at least tag each one with the version listed in its quark file.

On Mon, Apr 13, 2015 at 5:36 PM scztt notifications@github.com wrote:

After looking at this in a little more detail - I wonder if the
documentation should simply be changed? Supporting dependencies of the form
"quark@refspec" AND "quark" -> "refspec" doesn't seem necessary, and
opens the door to needless ambiguity (i.e. "quark@1.1" -> "1.2"). The
association form does everything we need just fine.


Reply to this email directly or view it on GitHub
#1377 (comment)
.

Member

crucialfelix commented Apr 13, 2015

The only reason it supports quark -> version is for backward compatibility.
Because all of the current quarks have their dependencies written that way.

But it can only be written that way in supercollider code. Its an
Association, not a formal syntax.

So I figured the directory.txt should not use this arrow thingy, it should
use something in common usage to other package managements systems.
which is @

And its natural to expect that if directory.txt allows @ then dependencies
should also allow it.

Why have two separate ways of specifying a quark and its version ?

But I didn't even implement support for @ in the dependencies and didn't
realize that I didn't until Miguel filed this bug.
IMO the dependencies list should support @ and that should be the
documented and recommended way to do it.

-> is for backwards compatibility

until somebody actually decides to write both @ and -> then we shouldn't
worry about it. who would do such a thing ?

So I will finish implementing @ in dependencies (and I found another bug in
that method)

and I will improve documentation to say that @ is the way to express it

and that -> is there for backwards compatibility

The bigger issue is that old quarks do not have tags, so we cannot check
them out by versions.
We could automatically tag them, but I think this is overkill.

We could at least tag each one with the version listed in its quark file.

On Mon, Apr 13, 2015 at 5:36 PM scztt notifications@github.com wrote:

After looking at this in a little more detail - I wonder if the
documentation should simply be changed? Supporting dependencies of the form
"quark@refspec" AND "quark" -> "refspec" doesn't seem necessary, and
opens the door to needless ambiguity (i.e. "quark@1.1" -> "1.2"). The
association form does everything we need just fine.


Reply to this email directly or view it on GitHub
#1377 (comment)
.

@scztt

This comment has been minimized.

Show comment
Hide comment
@scztt

scztt Apr 13, 2015

Contributor

Ah, didn't realize that -> was backwards only. Then this is right, apart from the bug regarding ref spec parsing. And if -> is effectively deprecated, then no need to protect people from using it incorrectly.

As per discussion here, the @ should already be split by the time we hit Quark.new - this should probably be done in Quarks:parseDependency.

👍 to adding a version tag to all existing quarks, especially if it meant that we could (semi-)enforce a requirement for tagged versions for any public quarks. Quarks in the public directory with no versions is just an open door to having things break randomly with no obvious cause.... (since you're getting whatever happens to be head whenever you install).

Contributor

scztt commented Apr 13, 2015

Ah, didn't realize that -> was backwards only. Then this is right, apart from the bug regarding ref spec parsing. And if -> is effectively deprecated, then no need to protect people from using it incorrectly.

As per discussion here, the @ should already be split by the time we hit Quark.new - this should probably be done in Quarks:parseDependency.

👍 to adding a version tag to all existing quarks, especially if it meant that we could (semi-)enforce a requirement for tagged versions for any public quarks. Quarks in the public directory with no versions is just an open door to having things break randomly with no obvious cause.... (since you're getting whatever happens to be head whenever you install).

crucialfelix added a commit to crucialfelix/supercollider that referenced this issue Apr 17, 2015

fix #1377 support "url@refspec" in Quark dependencies
Also fixes parsing a quark that is not found: reports and returns nil

@scztt scztt closed this in #1427 Apr 24, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment