unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Can we find a better idiom for unversioned packages?
@ 2021-08-31 19:57 Sarah Morgensen
  2021-08-31 21:20 ` Maxime Devos
                   ` (4 more replies)
  0 siblings, 5 replies; 34+ messages in thread
From: Sarah Morgensen @ 2021-08-31 19:57 UTC (permalink / raw)
  To: guix-devel

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.

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.

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.

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...)

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.

6. Not a direct complaint, but I feel like the overall package interface
was designed before straight-from-vcs unversioned packages were so
common, and so this idiom developed organically to work around that.

I do not have a specific solution in mind, but I think there must be
one.  I do have a few half-baked ideas, but I'm curious what we can all
come up with together.  Or maybe you'll just tell me I'm just being
awfully picky :)

[0] https://issues.guix.gnu.org/50286

--
Sarah


^ permalink raw reply	[flat|nested] 34+ messages in thread
* Re: Can we find a better idiom for unversioned packages?
@ 2021-09-03  5:51 Sarah Morgensen
  0 siblings, 0 replies; 34+ messages in thread
From: Sarah Morgensen @ 2021-09-03  5:51 UTC (permalink / raw)
  To: Maxime Devos; +Cc: guix-devel, Liliana Marie Prikler

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

>> > Not losing the revision is useful for things like 
>> > <https://issues.guix.gnu.org/50072>;, to be able to determine the old
>> > revision.  (That's not about inheriting packages though.)
>
>> Isn't that addressed by addressing the second point, though?  Like, if
>> you know the source location of the revision, you can read it back to
>> get the value itself (or possibly even access it as-is), no?
>
> Indeed!  The patch [0] addresses the second point.  With that patch,
> the proposed <extension-version> isn't required.  But also: some people
> (at least Sarah?) consider [0] a work-around, and if guix used something
> like <extended-version>, [0] wouldn't be necessary.
>
> It doesn't really matter to me what we'll end up using in guix
> in the long term, though in the short term, I would like something
> like [0] to be merged, as it is used by the (not-yet submitted, needs some
> cleanup, testing & rebasing) minetest updater, and it makes
> <https://issues.guix.gnu.org/50072> work in more cases.
>
> [0]: <https://issues.guix.gnu.org/50072>

Despite starting this thread, I do agree that your patch would be
helpful, at least in the short term, since I'd like to get git updating
working sooner rather than later :)

(There are a couple other usages of 'let' forms in packages, and your
patch could either help or hinder getting source locations for those,
depending on what semantics we want.  Very much an edge case, though.)

--
Sarah


^ permalink raw reply	[flat|nested] 34+ messages in thread
* Re: Can we find a better idiom for unversioned packages?
@ 2021-09-03 21:14 Sarah Morgensen
  2021-09-03 22:11 ` Liliana Marie Prikler
  0 siblings, 1 reply; 34+ messages in thread
From: Sarah Morgensen @ 2021-09-03 21:14 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: guix-devel, Xinglu Chen

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


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-09-08 22:39 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 19:57 Can we find a better idiom for unversioned packages? Sarah Morgensen
2021-08-31 21:20 ` Maxime Devos
2021-09-01 12:11   ` Xinglu Chen
2021-09-01 16:29     ` Maxime Devos
2021-09-01 13:33   ` Liliana Marie Prikler
2021-09-01 16:39     ` Maxime Devos
2021-09-01 18:34       ` Liliana Marie Prikler
2021-09-02 14:09         ` Maxime Devos
2021-09-02 14:20           ` Liliana Marie Prikler
2021-09-02 14:34             ` Maxime Devos
2021-09-01 19:48       ` Jonathan McHugh
2021-09-01 21:47         ` Liliana Marie Prikler
2021-09-02 13:32           ` Maxime Devos
2021-09-02  7:53         ` Jonathan McHugh
2021-09-02  9:25           ` Liliana Marie Prikler
2021-09-01 10:55 ` Xinglu Chen
2021-09-01 15:37   ` Leo Famulari
2021-09-01 16:50     ` Xinglu Chen
2021-09-02 16:51       ` Leo Famulari
2021-09-02 17:29         ` Leo Famulari
2021-09-03 16:11           ` Xinglu Chen
2021-09-03 16:35             ` Leo Famulari
2021-09-03 16:57               ` Leo Famulari
2021-09-03 20:03               ` Xinglu Chen
2021-09-04 21:00                 ` Leo Famulari
2021-09-08 21:15       ` Ludovic Courtès
2021-09-02 17:08 ` Leo Famulari
2021-09-08 21:28 ` Ludovic Courtès
2021-09-08 22:21 ` Jonathan McHugh
2021-09-08 22:38   ` Leo Famulari
  -- strict thread matches above, loose matches on Subject: below --
2021-09-03  5:51 Sarah Morgensen
2021-09-03 21:14 Sarah Morgensen
2021-09-03 22:11 ` Liliana Marie Prikler
2021-09-04 12:32   ` Taylan Kammer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).