From: Sarah Morgensen <iskarian@mgsn.dev>
To: Xinglu Chen <public@yoctocell.xyz>
Cc: "Ludovic Courtès" <ludo@gnu.org>, 50359@debbugs.gnu.org
Subject: [bug#50359] [PATCH 3/3] import: Add 'generic-git' updater.
Date: Thu, 16 Sep 2021 16:42:38 -0700 [thread overview]
Message-ID: <867dfghytt.fsf@mgsn.dev> (raw)
In-Reply-To: <87h7ekr8iw.fsf@yoctocell.xyz> (Xinglu Chen's message of "Thu, 16 Sep 2021 14:48:23 +0200 (10 hours, 1 minute, 2 seconds ago)")
[-- Attachment #1: Type: text/plain, Size: 5293 bytes --]
Xinglu Chen <public@yoctocell.xyz> writes:
>>> Maybe we could have a ‘release-tag-date-scheme?’ property, that way we
>>> could just try to match dates?
>>
>> That seems like it might be the only way to handle it in some cases (if
>> they have both versions and dates with a "." delimiter).
>
> It doesn’t have to be “.” delimiter though; if they both have the same
> delimiter it would be difficult to distinguish a version from a date,
> e.g., “1-2-3” vs “2021-03-23”.
Sure, but I haven't seen the former :)
>> (Though, we are actually interested in the *lack* of a date scheme.
>> If they use a date scheme now, other versions will be disregarded, so
>> we're fine; but if they use versions now and used a date scheme
>> before, the versions will be discarded.)
>
> I am not sure what you are trying to say, could you elaborate?
Just that the important case is disallowing dates when
'release-tag-date-scheme? is #f.
If the tags of a repo are:
12.1
12.2
13.0
13.4
2018.01.01
2018.05.05
and we do nothing, the 2018.05.05 tag will be selected. This is correct
if we do want dates, but incorrect if we don't (in which case we would
set 'tag-version-date-scheme? to #f to get the correct result).
>> Though it would be nice to see when such updates are available, is it
>> worth some bogus results? Are false positives better or false negatives
>> better?
>
> Hmm, good question! If in the future we have some kind of bot that
> automatically runs ‘guix refresh -u’, builds the updated package, and
> send a patch to the mailing list, not having false positives might be
> more important. We could also have a ‘disable-tag-updater?’ property to
> disable the updater for packages which gives false positive, or maybe
> that will result in to many properties.
For these packages, it would probably easier to just use the existing
tag- properties. In fact, instead of this or the date-scheme above,
a 'tag-version-regex' would cover both cases.
In fact, we could replace 'tag-version-delimiter' with
'tag-version-regex' and instead provide convencience functions such as
(untested):
(define (version-regex delim)
(let ((delim-rx (regexp-quote delim)))
(string-append "([[:digit:]][^" delim-rx "[:punct:]]*"
"(" delim-rx "[^[:punct:]" delim-rx "]+)"
(if (string=? delim-rx "") "*" "+"))))
(define* (version-date-regex (delim "."))
(let ((delim-rx (regexp-quote delim)))
(string-append "([0-9]{4}" delim-rx "(0[1-9]|11|12)"
delim-rx "(0[1-9]|[1-2][0-9])")))
WDYT?
>> Unless you/we want to pursue one or both of the above changes now, the
>> latest patch LGTM (modulo my nits).
>
> I would prefer to wait a bit with the improvements mentioned above. The
> current patch has been in the works a week or two already, so it’s
> probably a good idea to get it merged, and try to solve the less
> important issues later. :-)
Sounds good to me, then!
>> I discovered that this can segfault unless 'remote-disconnect' and
>> possibly 'repository-close!' are called *after* copying the data out.
>> I've attached a diff for this.
>
> I don’t see a diff attached; maybe you forgot? :-)
>
I've actually attached it this time :)
>>> +
>>> +(define (git-package? package)
>>> + "Whether the origin of PACKAGE is a Git repostiory."
>>
>> "Return true if PACKAGE is..."
>
> “PACKAGE is a Git repository.” doesn’t really sound right, maybe “if
> PACKAGE is hosted on a Git repository”?'
Sorry, yes, that's what I meant, or "Return true if the origin..."; I
was just suggesting making it a full sentence.
>> I tested this updater on all packages in .scm files starting with f
>> through z, and I found the following packages with possibly bogus
>> updates:
>>
>> --8<---------------cut here---------------start------------->8---
>> javaxom
>
> I assume you meant ‘java-xom’ :-)
>
> That’s a weird scheme; setting the delimiter to “.” doesn’t help since
> it thinks that “127” is greater than “1.3.7”.
'tag-version-regex would allow fixing this ;)
>
>> luakit
>> ocproxy
>> pitivi
>
> ‘pitivi’ has a pretty weird version string to begin with; it may be
> better to change it to the date: “0.999.0-2021-05.0” -> “2021-05.0”.
>
>> eid-mw
>> libhomfly
>> gnuradio
>> welle-io
>
> Setting the delimiter to "." fixes the issue.
>
>> racket-minimal
>
> Setting the prefix to "v" fixes this.
>
>> milkytracker
>> cl-portal
>> kodi-cli
>> openjdk
>> java-bouncycastle
>> hurd
>> opencsg
>
> Setting the suffix to "-release" fixes this.
>
>> povray
>> gpsbabel
>
> Setting the prefix to "gpsbabel_" fixes this.
>
>> go
>> stepmania
>> ocaml-mcl
>>
>> many minetest packages (minetest will have its own updater, though)
>>
>> ocaml4.07-core-kernel, ocamlbuild and many other ocaml packages
>> (they seem to be covered by the github updater)
>> --8<---------------cut here---------------end--------------->8---
I'm glad to see that these are easily fixed with the properties, though!
That's some good validation.
Now I just have to give the (guix upstream) some attention...
--
Sarah
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix undeterministic segfaults in remote-refs. --]
[-- Type: text/x-patch, Size: 1291 bytes --]
diff --git a/guix/git.scm b/guix/git.scm
index dc3d3afd02..bbff4fc890 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -593,6 +593,11 @@ is true, limit to only refs/tags."
(and (ref? ref)
(or (not tags?) (tag? ref))))
+ (define (remote-head->ref remote)
+ (let ((name (remote-head-name remote)))
+ (and (include? name)
+ name)))
+
(with-libgit2
(call-with-temporary-directory
(lambda (cache-directory)
@@ -600,14 +605,13 @@ is true, limit to only refs/tags."
;; Create an in-memory remote so we don't touch disk.
(remote (remote-create-anonymous repository url)))
(remote-connect remote)
- (remote-disconnect remote)
- (repository-close! repository)
-
- (filter-map (lambda (remote)
- (let ((name (remote-head-name remote)))
- (and (include? name)
- name)))
- (remote-ls remote)))))))
+
+ (let* ((remote-heads (remote-ls remote))
+ (refs (filter-map remote-head->ref remote-heads)))
+ ;; Wait until we're finished with the repository before closing it.
+ (remote-disconnect remote)
+ (repository-close! repository)
+ refs))))))
\f
;;;
next prev parent reply other threads:[~2021-09-16 23:43 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-03 15:50 [bug#50359] [PATCH] import: Add 'generic-git' updater Xinglu Chen
2021-09-05 0:19 ` Sarah Morgensen
2021-09-05 1:03 ` Sarah Morgensen
2021-09-05 10:36 ` Xinglu Chen
2021-09-06 5:40 ` Sarah Morgensen
2021-09-06 12:20 ` Xinglu Chen
2021-09-07 1:00 ` Sarah Morgensen
2021-09-07 19:13 ` Xinglu Chen
2021-09-08 18:28 ` Xinglu Chen
2021-09-10 8:36 ` Ludovic Courtès
2021-09-10 13:23 ` Xinglu Chen
2021-09-05 13:11 ` Xinglu Chen
2021-09-06 3:14 ` Sarah Morgensen
2021-09-10 16:20 ` [bug#50359] [PATCH 0/3] " Xinglu Chen
2021-09-10 16:21 ` [bug#50359] [PATCH 1/3] tests: git: Don't read from the users global Git config file Xinglu Chen
2021-09-10 16:21 ` [bug#50359] [PATCH 2/3] tests: git: Make 'tag' directive non-interactive Xinglu Chen
2021-09-13 8:03 ` Ludovic Courtès
2021-09-10 16:21 ` [bug#50359] [PATCH 3/3] import: Add 'generic-git' updater Xinglu Chen
2021-09-13 8:07 ` Ludovic Courtès
2021-09-16 9:09 ` Sarah Morgensen
2021-09-16 12:48 ` Xinglu Chen
2021-09-16 23:42 ` Sarah Morgensen [this message]
2021-09-17 7:48 ` Xinglu Chen
2021-09-17 8:04 ` [bug#50359] [PATCH v3 0/3] " Xinglu Chen
2021-09-17 8:04 ` [bug#50359] [PATCH v3 1/3] tests: git: Don't read from the users global Git config file Xinglu Chen
2021-09-17 8:04 ` [bug#50359] [PATCH v3 2/3] tests: git: Make 'tag' directive non-interactive Xinglu Chen
2021-09-17 8:04 ` [bug#50359] [PATCH v3 3/3] import: Add 'generic-git' updater Xinglu Chen
2021-09-18 17:47 ` bug#50359: [PATCH v3 0/3] " Ludovic Courtès
2021-09-15 8:44 ` [bug#50359] [PATCH 3/3] import: " iskarian
2021-09-15 11:59 ` Xinglu Chen
2021-09-16 9:46 ` Sarah Morgensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=867dfghytt.fsf@mgsn.dev \
--to=iskarian@mgsn.dev \
--cc=50359@debbugs.gnu.org \
--cc=ludo@gnu.org \
--cc=public@yoctocell.xyz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.