Re: Can we find a better idiom for unversioned packages?

[Sarah Morgensen <iskarian@mgsn.dev>]

Liliana,

Thanks for the criticism!

Liliana Marie Prikler <leo.prikler@student.tugraz.at> writes:

> Hi
>
> Am Dienstag, den 31.08.2021, 23:20 +0200 schrieb Maxime Devos:
>> Sarah Morgensen schreef op di 31-08-2021 om 12:57 [-0700]:
>> > Hello Guix,
>> > 
>> > Currently, there are about 1500 packages defined like this:
>> > 
>> > --8<---------------cut here---------------start------------->8---
>> > (define-public sbcl-feeder
>> >   (let ((commit "b05f517d7729564575cc809e086c262646a94d34")
>> >         (revision "1"))
>> >     (package
>> >       [...])))
>> > --8<---------------cut here---------------end--------------->8---
>> > 
>> > I feel like there are some issues with this idiom (in no particular
>> > order):
>> > 
>> > 1. When converting between this idiom and regularly versioned
>> > packages, the git diff shows the whole package changing because of
>> > the indentation change.
> If you are worried about that in a frequently changing package, you
> could set both to *unspecified* or #f instead, which would cause any
> reference to them in a string manipulation context to fail.  I don't
> think that such transitions are too frequent, though, as the point is
> rather to discourage them where not absolutely necessary and to use
> upstream releases instead.

To be clear, this point was regarding "git blame", not any programmatic
manipulation.

>> > 2. We cannot get at the source location for the definition of
>> > 'commit' or 'revision'.  This would be useful for updating these
>> > packages with `guix refresh -u`.  There is a proposed patch [0] to
>> > work around this, but it *is* a workaround.
> Other versioning idioms would also be workarounds, wouldn't they?
>
>> > 3. Packages inheriting from it lose the definitions.  For actual
>> > fields, we have e.g. `(package-version this-package)`, but we have
>> > no equivalent for these.
> What purpose would extracting those serve however?
>
>> > 4. Horizontal space is at a premium, and an extra two spaces here
>> > and there add up.  (Personally, I think we could do with a
>> > define-public-package macro to save another two spaces, but that's
>> > for another day...)
> The worst offenders in horizontal space typically come from the
> packages themselves and going from 79 to 81 in select outliers is AFAIU
> acceptable.
>
>> > 5. The closest thing we have to a standardized way of generating
>> > versions for these packages is `(version (git-version "0.0.0"
>> > revision commit))`.  We can do better than that boilerplate.
> This concerns packages without a public release, many of which never
> make one in the belief that commit hashes are valid versions (they are
> not).

You're largely correct, in that all these reasons are minor.  I feel
that they do add up, though... and, IMO, the current usage of 'let' just
feels inelegant.

> Perhaps we could make the point that origins can be versioned just like
> packages can and should think about where the version field really
> belongs.  I think having a per-origin version field and making it so
> that the package defaults to the origin version unless the package
> itself has a version specified, might be a better solution
> philosophically speaking.  Technically, it would only solve 1, 4 and 5,
> but there's a separate workaround for 2 and I don't really think 3 to
> be that big of an issue.

I like this idea.  It makes sense, in that the source does have an
inherent version, whether that's a blessed version number or some
identifier picked by the packager.  It keeps related values together.

(As you predicted, this does break down when multiple units of software
are packaged together and their versions don't necessarily match the
version of the source-as-a-whole, but being able to override the version
(as you suggested) should be sufficient.)

A minor downside of this method is that developers may have to look at
the source field to determine the package version if it's not directly
specified in the package (programmatic access would still work).
Similarly, anything which parses the raw sexps (such as committer.scm)
would have to be modified to accommodate.

Some ideas....

1. Version-first

(origin
  (version (vcs-version (base "2.13.3")))
  ;; or: (version "2.13.3")
  ;; (would require special-casing, but url-fetch uris would not need to
  ;; be modified)
  (method git-fetch)
  (uri (git-reference
        (url "https://github.com/git-lfs/git-lfs")
        (commit (string-append "v" version))))
        ;; or: (commit (version->tag version #:prefix "v"))
  (file-name (git-file-name name version))
  (sha256
   (base32
    "0r7dmqhkhz91d3n7qfpny483x8f1n88yya22j2fvx75rgg33z2sg")))

Or, if we encode the information a la #50359 [0], but in 'vcs-version':

(origin
  (version (vcs-version
            (base "2.13.3")
            (tag-prefix "v")))
  (method git-fetch)
  (uri (git-reference
        (url "https://github.com/git-lfs/git-lfs")
        (commit (vcs-version->git-commit version))))
  (file-name (git-file-name name version))
  (sha256
   (base32
    "0r7dmqhkhz91d3n7qfpny483x8f1n88yya22j2fvx75rgg33z2sg")))

In some ways it makes more sense to put e.g. tag-prefix in vcs-version,
rather than package properties, because the transformation can
occasionally vary between versions.

(origin
  (method git-fetch)
  (version (vcs-version
            (base "0.12.9")
            (identifier "e41b504dca90a25e9be27f296da7ce22e5782893")
            (revision "1")))
  (uri (git-reference
        (url "https://github.com/nosarthur/gita")
        (commit (vcs-version->git-commit version))))
  (file-name (git-file-name name version))
  (sha256
   (base32
    "1k03zgcbhl91cgyh4k7ywyjp00y63q4bqbimncqh5b3lni8l8j5l")))

Another advantage to this approach is that a potential git updater only
has to change the 'version' record.

2. Reference-first

(define* (git-ref->version ref #:optional base (revision "0")
                           #:key prefix suffix)
  (if base
    (vcs-version
     (base base)
     (identifier (git-reference-commit ref))
     (revision revision))
    (let ((prefix-len (or (and=> prefix string-length) 0))
          (suffix-len (or (and=> suffix string-length) 0)))
      (vcs-version
       (base (string-drop (string-drop-right ref suffix-len) prefix-len))
       (tag-prefix prefix)
       (tag-suffix suffix)))))

(origin
  (method git-fetch)
  (uri (git-reference
        (url "https://github.com/git-lfs/git-lfs")
        (commit "v2.13.3")))
  (version (git-ref->version uri #:prefix "v"))
  (file-name (git-file-name name version))
  (sha256
   (base32
    "0r7dmqhkhz91d3n7qfpny483x8f1n88yya22j2fvx75rgg33z2sg")))

(origin
  (method git-fetch)
  (uri (git-reference
        (url "https://github.com/nosarthur/gita")
        (commit "e41b504dca90a25e9be27f296da7ce22e5782893")))
  (version (git-ref->version uri "0.12.9" "1"))
  (file-name (git-file-name name version))
  (sha256
   (base32
    "1k03zgcbhl91cgyh4k7ywyjp00y63q4bqbimncqh5b3lni8l8j5l")))

Hmmm. I think this method falls short of the previous because we're
forced to work backwards from the tag, and it splits the information
relevant for updating between 'version' and 'uri'.

Perhaps 'vcs-version' should actually be separated into 'tagged-version'
and 'improper-version' or similar?

[0] <https://issues.guix.gnu.or/50359> Add 'generic-git' updater.

--
Sarah