unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#56398: (guix git) fails to check out repos with nested submodules
@ 2022-07-05 15:02 Ludovic Courtès
  2022-07-07 21:35 ` André Batista
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2022-07-05 15:02 UTC (permalink / raw)
  To: 56398

Hello,

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

--8<---------------cut here---------------start------------->8---
$ ./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
--8<---------------cut here---------------end--------------->8---

Ludo’.




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-05 15:02 bug#56398: (guix git) fails to check out repos with nested submodules Ludovic Courtès
@ 2022-07-07 21:35 ` André Batista
  2022-07-08  2:45   ` André Batista
  2022-07-08  8:26   ` Ludovic Courtès
  0 siblings, 2 replies; 10+ messages in thread
From: André Batista @ 2022-07-07 21:35 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56398

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:

--8<---------------cut here---------------start------------->8---
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

--8<---------------cut here---------------end--------------->8---




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-07 21:35 ` André Batista
@ 2022-07-08  2:45   ` André Batista
  2022-07-08 10:17     ` bokr
  2022-07-08  8:26   ` Ludovic Courtès
  1 sibling, 1 reply; 10+ messages in thread
From: André Batista @ 2022-07-08  2:45 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56398

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?




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-07 21:35 ` André Batista
  2022-07-08  2:45   ` André Batista
@ 2022-07-08  8:26   ` Ludovic Courtès
  2022-08-04 11:43     ` André Batista
  1 sibling, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2022-07-08  8:26 UTC (permalink / raw)
  To: André Batista; +Cc: 56398

Hi!

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

> 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:

--8<---------------cut here---------------start------------->8---
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.
--8<---------------cut here---------------end--------------->8---

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


Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
--8<---------------cut here---------------end--------------->8---

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

--8<---------------cut here---------------start------------->8---
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
--8<---------------cut here---------------end--------------->8---

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




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-08  2:45   ` André Batista
@ 2022-07-08 10:17     ` bokr
  2022-08-04 12:01       ` André Batista
  0 siblings, 1 reply; 10+ messages in thread
From: bokr @ 2022-07-08 10:17 UTC (permalink / raw)
  To: André Batista; +Cc: Ludovic Courtès, 56398

Hi,

On +2022-07-07 23:45:35 -0300, André Batista wrote:
> 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? 

[1]:    <https://lwn.net/Articles/848935/>

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




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-08  8:26   ` Ludovic Courtès
@ 2022-08-04 11:43     ` André Batista
  2022-08-04 11:59       ` Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: André Batista @ 2022-08-04 11:43 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56398

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

Hi and I'm sorry for the delay..

sex 08 jul 2022 às 10:26:40 (1657286800), ludovic.courtes@inria.fr enviou:
> 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!

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

--8<---------------cut here---------------start------------->8---
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)))
--8<---------------cut here---------------end--------------->8---

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!

---

[-- Attachment #2: git-scm.patch --]
[-- Type: text/plain, Size: 2709 bytes --]

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

Fixes <https://issues.guix.gnu.org/56398>.
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(-)

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


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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-08-04 11:43     ` André Batista
@ 2022-08-04 11:59       ` Ludovic Courtès
  2022-08-05 20:10         ` André Batista
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2022-08-04 11:59 UTC (permalink / raw)
  To: André Batista; +Cc: 56398

Hello,

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

> 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?

> +                (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’.




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-07-08 10:17     ` bokr
@ 2022-08-04 12:01       ` André Batista
  0 siblings, 0 replies; 10+ messages in thread
From: André Batista @ 2022-08-04 12:01 UTC (permalink / raw)
  To: bokr; +Cc: Ludovic Courtès, 56398

Hi Bengt!

sex 08 jul 2022 às 12:17:59 (1657293479), bokr@bokr.com enviou:
> 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!

> 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!




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-08-04 11:59       ` Ludovic Courtès
@ 2022-08-05 20:10         ` André Batista
  2022-08-05 22:40           ` André Batista
  0 siblings, 1 reply; 10+ messages in thread
From: André Batista @ 2022-08-05 20:10 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56398

qui 04 ago 2022 às 13:59:20 (1659632360), ludovic.courtes@inria.fr enviou:
> 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?

> 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?

> 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!

[1] https://git-scm.com/docs/git-submodule
[2] https://git-scm.com/docs/gitsubmodules




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

* bug#56398: (guix git) fails to check out repos with nested submodules
  2022-08-05 20:10         ` André Batista
@ 2022-08-05 22:40           ` André Batista
  0 siblings, 0 replies; 10+ messages in thread
From: André Batista @ 2022-08-05 22:40 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56398

sex 05 ago 2022 às 17:10:50 (1659730250), nandre@riseup.net enviou:
> 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!




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

end of thread, other threads:[~2022-08-05 22:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-05 15:02 bug#56398: (guix git) fails to check out repos with nested submodules Ludovic Courtès
2022-07-07 21:35 ` André Batista
2022-07-08  2:45   ` André Batista
2022-07-08 10:17     ` bokr
2022-08-04 12:01       ` André Batista
2022-07-08  8:26   ` Ludovic Courtès
2022-08-04 11:43     ` André Batista
2022-08-04 11:59       ` Ludovic Courtès
2022-08-05 20:10         ` André Batista
2022-08-05 22:40           ` André Batista

Code repositories for project(s) associated with this 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).