(guix git) fails to check out repos with nested submodules

  • Open
  • quality assurance status badge
Details
3 participants
  • bokr
  • Ludovic Courtès
  • André Batista
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
normal
L
L
Ludovic Courtès wrote on 5 Jul 2022 17:02
(address . bug-guix@gnu.org)
87sfnf4n7c.fsf@inria.fr
Hello,

It seems that ‘update-cached-checkout’ fails to handle nested Git
submodules, when a submodule itself has submodules:

Toggle snippet (52 lines)
$ ./pre-inst-env guix refresh python-pytorch
gnu/packages/machine-learning.scm:2872:13: python-pytorch would be upgraded from 1.11.0 to 1.12.0
$ ./pre-inst-env guix refresh -u python-pytorch
Backtrace:
In ice-9/eval.scm:
619:8 19 (_ #(#(#<directory (guile-user) 7f5a34cc2c80>)))
In guix/ui.scm:
2238:7 18 (run-guix . _)
2201:10 17 (run-guix-command _ . _)
In ice-9/boot-9.scm:
1752:10 16 (with-exception-handler _ _ #:unwind? _ # _)
1752:10 15 (with-exception-handler _ _ #:unwind? _ # _)
In guix/store.scm:
659:37 14 (thunk)
2168:25 13 (run-with-store #<store-connection 256.99 7f5a212475f0> …)
In guix/scripts/refresh.scm:
560:16 12 (_ _)
In srfi/srfi-1.scm:
634:9 11 (for-each #<procedure 7f5a20eda4e0 at guix/scripts/ref…> …)
In guix/scripts/refresh.scm:
309:22 10 (update-package _ #<package python-pytorch@1.11.0 gnu/…> …)
In guix/upstream.scm:
475:10 9 (package-update/git-fetch _ _ #<<upstream-source> pack…> …)
In guix/git.scm:
550:8 8 (latest-repository-commit #<store-connection 256.99 7f…> …)
472:7 7 (update-cached-checkout _ #:ref _ #:recursive? _ # _ # _ …)
In srfi/srfi-1.scm:
634:9 6 (for-each #<procedure 7f5a265c7f30 at guix/git.scm:321…> …)
In guix/git.scm:
333:20 5 (_ _)
In srfi/srfi-1.scm:
634:9 4 (for-each #<procedure 7f5a266631e0 at guix/git.scm:321…> …)
In guix/git.scm:
325:16 3 (_ _)
In git/bindings.scm:
77:2 2 (raise-git-error _)
In ice-9/boot-9.scm:
1685:16 1 (raise-exception _ #:continuable? _)
1685:16 0 (raise-exception _ #:continuable? _)

ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Git error: failed to resolve path '/home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/third-party/googletest/.git': No such file or directory
$
$ ls /home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/third-party/
ls: cannot access '/home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/third-party/': No such file or directory
$ ls /home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/
cmake/ CODE_OF_CONDUCT.md docs/ LICENSE tools/
CMakeLists.txt CONTRIBUTING.md gloo/ README.md
$ ls /home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/.gitmodules
/home/ludo/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/.gitmodules

Ludo’.
A
A
André Batista wrote on 7 Jul 2022 23:35
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
YsdRrr/dBytmSyqQ@andel
Hi!

ter 05 jul 2022 às 17:02:31 (1657051351), ludovic.courtes@inria.fr enviou:
Toggle quote (3 lines)
> It seems that ‘update-cached-checkout’ fails to handle nested Git
> submodules, when a submodule itself has submodules:

I think this may be actually a bug upstream. When refreshing, guix is
able to recursively upgrade submodules under 'thirdparty/fbgemm' just
fine, but it fails on thirdparty/gloo because the path declared on
its .gitmodules does not exist on the repository:

Toggle snippet (52 lines)
user@guix /src/guix.git$ cat ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/.gitmodules
[submodule "third-party/googletest"]
path = third-party/googletest
url = https://github.com/google/googletest.git
user@guix /src/guix.git$ ls ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/gloo/
cmake/ CMakeLists.txt CODE_OF_CONDUCT.md CONTRIBUTING.md docs/ gloo/ LICENSE README.md tools/
user@guix /src/guix.git$ cat ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/fbgemm/.gitmodules
[submodule "third_party/asmjit"]
path = third_party/asmjit
url = https://github.com/asmjit/asmjit.git
[submodule "third_party/cpuinfo"]
path = third_party/cpuinfo
url = https://github.com/pytorch/cpuinfo
[submodule "third_party/googletest"]
path = third_party/googletest
url = https://github.com/google/googletest
user@guix /src/guix.git$ ls ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/third_party/fbgemm/third_party/
asmjit/ asmjit.BUILD cpuinfo/ cpuinfo.BUILD googletest/
user@guix ~$ cat ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/.git/modules/third_party/gloo/config
[core]
bare = false
repositoryformatversion = 0
filemode = true
logallrefupdates = true
worktree = ../../../../third_party/gloo/
[remote "origin"]
url = https://github.com/facebookincubator/gloo
fetch = +refs/heads/*:refs/remotes/origin/*
[branch "main"]
remote = origin
merge = refs/heads/main
user@guix ~$ cat ~/.cache/guix/checkouts/orwu3gj6brdg2ui6r2tl5me7plucokpvqvgbhe2yuo6jc2tovvha/.git/modules/third_party/fbgemm/config
[core]
bare = false
repositoryformatversion = 0
filemode = true
logallrefupdates = true
worktree = ../../../../third_party/fbgemm/
[remote "origin"]
url = https://github.com/pytorch/fbgemm
fetch = +refs/heads/*:refs/remotes/origin/*
[branch "main"]
remote = origin
merge = refs/heads/main
[submodule "third_party/asmjit"]
url = https://github.com/asmjit/asmjit.git
[submodule "third_party/cpuinfo"]
url = https://github.com/pytorch/cpuinfo
[submodule "third_party/googletest"]
url = https://github.com/google/googletest

A
A
André Batista wrote on 8 Jul 2022 04:45
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
YseaTzCqYWQDaASs@andel
qui 07 jul 2022 �s 18:35:42 (1657229742), nandre@riseup.net enviou:
Toggle quote (2 lines)
> I think this may be actually a bug upstream. (...)

I mean, we could change the submodule-update fetch-options to allow
for repository initialization when it's absent, but does it make
sense considering we would just remove it when building the package
and use package inputs' version instead?
L
L
Ludovic Courtès wrote on 8 Jul 2022 10:26
(name . André Batista)(address . nandre@riseup.net)(address . 56398@debbugs.gnu.org)
87pmigxb5r.fsf@inria.fr
Hi!

André Batista <nandre@riseup.net> skribis:

Toggle quote (11 lines)
> Hi!
>
> ter 05 jul 2022 às 17:02:31 (1657051351), ludovic.courtes@inria.fr enviou:
>> It seems that ‘update-cached-checkout’ fails to handle nested Git
>> submodules, when a submodule itself has submodules:
>
> I think this may be actually a bug upstream. When refreshing, guix is
> able to recursively upgrade submodules under 'thirdparty/fbgemm' just
> fine, but it fails on thirdparty/gloo because the path declared on
> its .gitmodules does not exist on the repository:

Oh indeed. The bug probably doesn’t have to do with nested submodules;
it manifests when checking out that gloo repo:

Toggle snippet (8 lines)
scheme@(guix git)> (update-cached-checkout "https://github.com/facebookincubator/gloo" #:recursive? #t)
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Git error: failed to resolve path '/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest/.git': No such file or directory


Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.

If we do this:

Toggle snippet (12 lines)
scheme@(guix git)> (define r (repository-open "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/"))
scheme@(guix git)> (define mod (submodule-lookup r "third-party/googletest"))
scheme@(guix git)> mod
$13 = #<git-submodule 17d1220>
scheme@(guix git)> (submodule-update $13)
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Git error: failed to resolve path '/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest/.git': No such file or directory


Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue.

… the ‘submodule-update’ bit looks like this:

Toggle snippet (29 lines)
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", {st_mode=S_IFREG|0644, st_size=116, ...}, 0) = 0
access("/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", F_OK) = 0
access("/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", R_OK) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", {st_mode=S_IFREG|0644, st_size=116, ...}, 0) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", {st_mode=S_IFREG|0644, st_size=116, ...}, 0) = 0
openat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", O_RDONLY|O_CLOEXEC) = 15
read(15, "[submodule \"third-party/googletest\"]\n\tpath = third-party/googletest\n\turl = https://github.com/google/googletest.git\n", 116) = 116
close(15) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.gitmodules", {st_mode=S_IFREG|0644, st_size=116, ...}, 0) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest", 0x7ffdd31497f0, 0) = -1 ENOENT (No such file or directory)
access("/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest/.git", F_OK) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/HEAD", {st_mode=S_IFREG|0644, st_size=21, ...}, 0) = 0
openat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/HEAD", O_RDONLY|O_CLOEXEC) = 15
read(15, "ref: refs/heads/main\n", 21) = 21
close(15) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/refs/heads/main", {st_mode=S_IFREG|0644, st_size=41, ...}, 0) = 0
openat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/refs/heads/main", O_RDONLY|O_CLOEXEC) = 15
read(15, "950c0e23819779a9e0c70b861db4c52b31d1d1b2\n", 41) = 41
close(15) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/HEAD", {st_mode=S_IFREG|0644, st_size=21, ...}, 0) = 0
openat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/HEAD", O_RDONLY|O_CLOEXEC) = 15
read(15, "ref: refs/heads/main\n", 21) = 21
close(15) = 0
newfstatat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/refs/heads/main", {st_mode=S_IFREG|0644, st_size=41, ...}, 0) = 0
openat(AT_FDCWD, "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/.git/refs/heads/main", O_RDONLY|O_CLOEXEC) = 15
read(15, "950c0e23819779a9e0c70b861db4c52b31d1d1b2\n", 41) = 41
close(15) = 0

Thus, looking at ‘git_submodule_update’ in libgit2, it looks as if this
condition was true:

(submodule_status & GIT_SUBMODULE_STATUS_WD_UNINITIALIZED)

Hmm, thoughts?

Ludo’.
B
B
bokr wrote on 8 Jul 2022 12:17
(name . André Batista)(address . nandre@riseup.net)
20220708101759.GA6315@LionPure
Hi,

On +2022-07-07 23:45:35 -0300, André Batista wrote:
Toggle quote (9 lines)
> qui 07 jul 2022 às 18:35:42 (1657229742), nandre@riseup.net enviou:
> > I think this may be actually a bug upstream. (...)
>
> I mean, we could change the submodule-update fetch-options to allow
> for repository initialization when it's absent, but does it make
> sense considering we would just remove it when building the package
> and use package inputs' version instead?
>

Have you seen this[1] re nested git tricks?


i.e., are you sure not to be used by some such attack?
(article was over a year ago, so there must be more on
related git problems by now?)

--
Regards,
Bengt Richter
A
A
André Batista wrote on 4 Aug 2022 13:43
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
Yuuw9eo8UAA2ZYE3@andel
Hi and I'm sorry for the delay..

sex 08 jul 2022 às 10:26:40 (1657286800), ludovic.courtes@inria.fr enviou:
Toggle quote (11 lines)
> If we do this:
>
> --8<---------------cut here---------------start------------->8---
> scheme@(guix git)> (define r (repository-open "/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/"))
> scheme@(guix git)> (define mod (submodule-lookup r "third-party/googletest"))
> scheme@(guix git)> mod
> $13 = #<git-submodule 17d1220>
> scheme@(guix git)> (submodule-update $13)
> ice-9/boot-9.scm:1685:16: In procedure raise-exception:
> Git error: failed to resolve path '/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest/.git': No such file or directory

Nice trick!

Toggle quote (11 lines)
> (...)
> access("/home/ludo/.cache/guix/checkouts/4pe5bqkmsmcros5oviwzvmnjlie6jyhx2dciznz7shwawckb32sq/third-party/googletest/.git", F_OK) = -1 ENOENT (No such file or directory)
> (...)
>
> Thus, looking at ‘git_submodule_update’ in libgit2, it looks as if this
> condition was true:
>
> (submodule_status & GIT_SUBMODULE_STATUS_WD_UNINITIALIZED)
>
> Hmm, thoughts?

Well, I guess ENOENT != GIT_ENOTFOUND and, in that case, I think we
are better off gracefully failing to update it than trying to "branch"
our local repo. The patch below tests for the directory's existence
before proceding with the update. With this patch applied I've managed
to refresh pytorch, its submodules and got the following code:

Toggle snippet (24 lines)
diff --git a/gnu/packages/machine-learning.scm b/gnu/packages/machine-learning.scm
index b19af8a1d5..174ba3d39b 100644
--- a/gnu/packages/machine-learning.scm
+++ b/gnu/packages/machine-learning.scm
@@ -2871,7 +2871,7 @@ (define-public xnnpack
(define-public python-pytorch
(package
(name "python-pytorch")
- (version "1.12.0")
+ (version "82782")
(source (origin
(method git-fetch)
(uri (git-reference
@@ -2881,7 +2881,7 @@ (define-public python-pytorch
(file-name (git-file-name name version))
(sha256
(base32
- "0pdqi91qzgyx947zv4pw2fdj9vpqvdhfzw1ydjd4mpqm8g5njgnz"))
+ "0zshqfqv3lcwyym3l8zx675chnhpxn14c4nr1c2b7ci1zis785va"))
(patches (search-patches "python-pytorch-system-libraries.patch"
"python-pytorch-runpath.patch"))
(modules '((guix build utils)))

Where "82782" is a ciflow/trunk reference to commit
700dba518be03ee0c0d6389162b5907a13838f49.

I've also thought of filtering the submodules list and passing the
resulting list instead, but came to conclude that it would clutter
the code instead of simplifying it.

I hope that helps!

---
From 10bbb79f87f3728c347e33a101add8cb740e9469 Mon Sep 17 00:00:00 2001
In-Reply-To: <56398@debbugs.gnu.org>
References: <56398@debbugs.gnu.org>
From: =?UTF-8?q?Andr=C3=A9=20Batista?= <nandre@riseup.net>
Date: Thu, 4 Aug 2022 08:07:35 -0300
Subject: [PATCH] guix: git: Gracefully handle missing submodules when updating.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
To: debbugs@gnu.org

Reported by Ludovic Court�s <ludo@gnu.org>.

* guix/git.scm (update-submodules): Check if submodule directory
exists before trying to update it.
---
guix/git.scm | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/git.scm b/guix/git.scm
index 631bf577d3..d6a82fb86c 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -320,20 +320,22 @@ (define* (update-submodules repository
(fetch-options #f))
"Update the submodules of REPOSITORY, a Git repository object."
(for-each (lambda (name)
- (let ((submodule (submodule-lookup repository name)))
+ (let* ((submodule (submodule-lookup repository name))
+ (directory (string-append
+ (repository-working-directory repository)
+ "/" (submodule-path submodule))))
(format log-port (G_ "updating submodule '~a'...~%")
name)
- (submodule-update submodule
- #:fetch-options fetch-options)
-
- ;; Recurse in SUBMODULE.
- (let ((directory (string-append
- (repository-working-directory repository)
- "/" (submodule-path submodule))))
- (with-repository directory repository
- (update-submodules repository
- #:fetch-options fetch-options
- #:log-port log-port)))))
+ (if (file-exists? directory)
+ ((lambda ()
+ (submodule-update submodule
+ #:fetch-options fetch-options)
+
+ ;; Recurse in SUBMODULE.
+ (with-repository directory repository
+ (update-submodules repository
+ #:fetch-options fetch-options
+ #:log-port log-port)))))))
(repository-submodules repository)))
(define-syntax-rule (false-if-git-not-found exp)
--
2.36.0
L
L
Ludovic Courtès wrote on 4 Aug 2022 13:59
(name . André Batista)(address . nandre@riseup.net)(address . 56398@debbugs.gnu.org)
87h72smd7r.fsf@inria.fr
Hello,

André Batista <nandre@riseup.net> skribis:

Toggle quote (6 lines)
> Well, I guess ENOENT != GIT_ENOTFOUND and, in that case, I think we
> are better off gracefully failing to update it than trying to "branch"
> our local repo. The patch below tests for the directory's existence
> before proceding with the update. With this patch applied I've managed
> to refresh pytorch, its submodules and got the following code:

But you got a wrong hash I presume, because submodule weren’t actually
checked out, right?

Toggle quote (5 lines)
> + (if (file-exists? directory)
> + ((lambda ()
> + (submodule-update submodule
> + #:fetch-options fetch-options)

The problem is that it papers over an actual libgit2 bug.

I think we should instead report it upstream. Do you feel like doing
it? I guess we’d need to give them the C version of the three-line
snippet I gave earlier.

WDYT?

Thanks,
Ludo’.
A
A
André Batista wrote on 4 Aug 2022 14:01
(address . bokr@bokr.com)
Yuu1EWcM5hhMHE+f@andel
Hi Bengt!

sex 08 jul 2022 �s 12:17:59 (1657293479), bokr@bokr.com enviou:
Toggle quote (4 lines)
> Have you seen this[1] re nested git tricks?
>
> [1]: <https://lwn.net/Articles/848935/>

No, I had missed that, thanks for pointing that out!

Toggle quote (2 lines)
> i.e., are you sure not to be used by some such attack?

However I think this git issue is orthogonal to the current one.

First, inits, clones and checkouts are key git features, so it's
up to git to make sure its subcommands will not execute code by
mistake.

Second, to exploit it, the attacker would have to make themselves
very visible by maintaining a public malicious repo which would be
bound to be flagged.

And lastly, guile-git uses libgit2, which is a different beast that
actually auto initializes submodules when updating, contrary to my
mistaken assumption to which you've replied. I thought
initialization implied directory creation, but it actually doesn't.

Cheers!
A
A
André Batista wrote on 5 Aug 2022 22:10
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
Yu15Ss+vIksxt0Z9@andel
qui 04 ago 2022 às 13:59:20 (1659632360), ludovic.courtes@inria.fr enviou:
Toggle quote (3 lines)
> But you got a wrong hash I presume, because submodule weren’t actually
> checked out, right?

The submodule was not checked out, sure, but is it the wrong hash
considering it's also not present on the remote repo?

Toggle quote (2 lines)
> The problem is that it papers over an actual libgit2 bug.

Does it though? I'm trying to wrap my head over this, but I'm somewhat
unsure that this isn't the intended - expected? - behaviour on this
case:

I've cloned gloo and issued 'git submodule status' and it returns
nothing. I've also issued 'git submodule update --init --recursive'
and it also does nothing.

The documentation for git-submodule init[1] says "Initialize the
submodules recorded in the index (which were _added and commited_
elsewhere)..." and it also says on 'add' subcommand "Add the given
repo as a submodule at the given path to the changeset to be commited
next to the current project..."

The documentation for gitsubmodules[2] says that submodules can take
the following forms:

- a .git directory, a working directory, a gitlink and a .gitmodules;
or
- a working directory with an embedded .git directory (...)

The description for libgit2's git_submodule_init (which would've been
called if the error was GIT_ENOTFOUND and we were initializing it)
only says "Copy submodule info into .git/config", which coincides with
what 'git submodule init' does. There is also a git_submodule_add_setup
routine which can be called to emulate what 'git submodule add'
would've done.

Finally, libgit2 provides a git_submodule_repo_init routine which would
"set up the subrepository for a submodule in preparation for clone",
instead of just copying the submodule info into .git/config as
git_submodule_init does.

So, I'm inclined to think that this is not a bug on libgit2, but a
result of gloo having misconfigured its submodule and it's not
surprising that 'git_submodule_update' is erroring out. If so, any
program using this library should decide what to do with the returned
error, right?

Toggle quote (6 lines)
> I think we should instead report it upstream. Do you feel like doing
> it? I guess we’d need to give them the C version of the three-line
> snippet I gave earlier.
>
> WDYT?

"Feel like doing it" is a stretch, but I'll gladly do it so that at
least we will know that they've considered this scenario and, if they
agree with you, we and other users may have a proper fix. Let's see if
I can conjure some C incantation.

Cheers!

A
A
André Batista wrote on 6 Aug 2022 00:40
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
Yu2cUfhpUdoUsxjY@andel
sex 05 ago 2022 �s 17:10:50 (1659730250), nandre@riseup.net enviou:
Toggle quote (10 lines)
> Does it though? I'm trying to wrap my head over this, but I'm somewhat
> unsure that this isn't the intended - expected? - behaviour on this
> case:
> (...)
> So, I'm inclined to think that this is not a bug on libgit2, but a
> result of gloo having misconfigured its submodule and it's not
> surprising that 'git_submodule_update' is erroring out. If so, any
> program using this library should decide what to do with the returned
> error, right?

On further thinking, I guess I see what you mean: there's not much to
be done in such a case and it would be better if libgit2 would
silently fail, mimicking what git itself does, otherwise this logic
would need to be reproduced everywhere else for no reason whatsoever.

Sorry for the noise!
A
A
André Batista wrote on 24 Nov 2022 16:17
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
Y3+K7cBeds6756u7@andel
Hi!

qui 04 ago 2022 às 13:59:20 (1659632360), ludovic.courtes@inria.fr enviou:
Toggle quote (4 lines)
> I think we should instead report it upstream. Do you feel like doing
> it? I guess we’d need to give them the C version of the three-line
> snippet I gave earlier.

Upstream issue #6433[1]

Apparently, GIT_SUBMODULE_STATUS_WD_UNINITIALIZED isn't actually set
in this scenario, only GIT_SUBMODULE_STATUS_IN_CONFIG.

B
B
bokr wrote on 25 Nov 2022 00:51
(name . André Batista)(address . nandre@riseup.net)
20221124235143.GA8148@LionPure
Hi,

On +2022-11-24 12:17:01 -0300, André Batista wrote:
Toggle quote (17 lines)
> Hi!
>
> qui 04 ago 2022 às 13:59:20 (1659632360), ludovic.courtes@inria.fr enviou:
> > I think we should instead report it upstream. Do you feel like doing
> > it? I guess we’d need to give them the C version of the three-line
> > snippet I gave earlier.
>
> Upstream issue #6433[1]
>
> Apparently, GIT_SUBMODULE_STATUS_WD_UNINITIALIZED isn't actually set
> in this scenario, only GIT_SUBMODULE_STATUS_IN_CONFIG.
>
> 1. https://github.com/libgit2/libgit2/issues/6433
>
>
>

Wondering if this[1] is all history in gnu/guix-land:


Wherein it says

Toggle snippet (10 lines)
The problem has been patched in the versions published on
April 14th, 2020, going back to v2.17.x. Anyone wishing to
backport the change further can do so by applying commit
9a6bbee (the full release includes extra checks for git
fsck, but that commit is sufficient to protect clients
against the vulnerability). The patched versions are:
2.17.4, 2.18.3, 2.19.4, 2.20.3, 2.21.2, 2.22.3, 2.23.2,
2.24.2, 2.25.3, 2.26.1.

Is there an automated tool to answer the question,
"What executables (command line directly, or indirectly (including
config-directed interpretation)) does my system contain
that have known vulnerabilities?"

BTW: Newsflash: :)
RMS paranoia now dernier-cri[3] as cited in [2]

Something[3] to get (more) serious about for gnu/guix?
--
Regards,
Bengt Richter
A
A
André Batista wrote on 28 Nov 2022 16:41
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)(address . 56398@debbugs.gnu.org)
Y4TWxYdnOos18AL8@andel
qui 24 nov 2022 �s 12:17:19 (1669303039), nandre@riseup.net enviou:
Toggle quote (2 lines)
> Upstream issue #6433

Fixed on upstream commit 936b184e7494158c20e522981f4a324cac6ffa47
L
L
Ludovic Courtès wrote on 28 Nov 2022 17:57
(name . André Batista)(address . nandre@riseup.net)(address . 56398@debbugs.gnu.org)
87tu2jauf2.fsf@inria.fr
Hi André,

André Batista <nandre@riseup.net> skribis:

Toggle quote (5 lines)
> qui 24 nov 2022 às 12:17:19 (1669303039), nandre@riseup.net enviou:
>> Upstream issue #6433
>
> Fixed on upstream commit 936b184e7494158c20e522981f4a324cac6ffa47

Excellent. Let’s close this bug once we’ve updated the ‘libgit2’
package to a version that includes this fix.

Thanks for following up!

Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 56398@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 56398
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch