bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#43893: [PATCH v2] maint: update-guix-package: Prevent accidentally b


From: Maxim Cournoyer
Subject: bug#43893: [PATCH v2] maint: update-guix-package: Prevent accidentally breaking guix pull.
Date: Wed, 14 Oct 2020 00:10:07 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Hello Marius,

And thanks for the review!

[...]

>> +(define (git-add-worktree directory commit-ish)
>> +  "Create a new git worktree at DIRECTORY, detached on commit COMMIT-ISH."
>> +  (invoke "git" "worktree" "add" "--detach" directory commit-ish))
>
> Is it feasible to use Guile-Git here (given appropriate bindings)?

I had a cursory look at the guile-git sources, and it seems to miss at
least bindings for manipulating worktrees, and I'm not sure how we could
get a list of all the remotes and check their URLs to find the remote
used to push to Savannah.  Supposing we'd be able to get that remote, I
also don't know how we could query if its master branch contains a given
commit (I imagine it's doable, but it's not documented so it takes time
to figure it out :-).

[...]

>> -                              (string-append (getcwd) "/" root))
>> +(define (commit-already-pushed? remote commit)
>> +  "True if COMMIT is found in the REMOTE repository."
>> +  (not (string-null? (with-input-pipe-to-string
>> +                      "git" "branch" "-r" "--contains" commit
>> +                      (string-append remote "/master")))))
>
> ...because parsing git CLI output is error-prone and "ugly" (IMO).  But
> not a strong opinion.

I agree; but for the time being we don't have an another option.  I'd be
happy to be proven wrong.

[...]

>> +         (lambda ()
>> +           (invoke "git" "worktree" "prune")))))
>
> This is not great, because users (well, developers who run this script)
> may have worktrees that are temporarily inaccessible (e.g. on a USB
> drive or whatever).  Better to just leave the stale reference instead of
> potentially destroying users worktrees.

That's a good point.  I've improved the cleanup in v3 to only remove the
worktree it creates and no other.

> Perhaps the script could 'git clone --maxdepth=1' instead of creating
> a worktree?

I think you meant something like:

--8<---------------cut here---------------start------------->8---
$ git clone --branch the-branch --depth -1 %top-srcdir
--8<---------------cut here---------------end--------------->8---

That could work, but it's about 2x slower and more expensive than
creating a worktree (15975 syscalls vs 647, according to 'strace -c').

Thank you,

Maxim





reply via email to

[Prev in Thread] Current Thread [Next in Thread]