diff mbox series

[bug#55242,01/10] guix: import: Print the number of packages at the end.

Message ID 20220503111620.3139-1-attila@lendvai.name
State New
Headers show
Series [bug#55242,01/10] guix: import: Print the number of packages at the end. | expand

Commit Message

Attila Lendvai May 3, 2022, 11:16 a.m. UTC
---

this will be a series of patches that were needed to be able to
(mostly) successfully run these two imports (go-ethereum and
ethersphere/bee):

RUN=12
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethersphere/bee@v1.5.1 > >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/bee-run-${RUN}.log >&2)

RUN=36
clear && ./pre-inst-env guix import go -r --pin-versions github.com/ethereum/go-ethereum@v1.10.17 > >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.scm) 2> >(tee -a ~/workspace/guix/importing/go-ethereum-run-${RUN}.log >&2)

note that i only have a mediocre knowledge of the golang
infrastructure, so this should be reviewed by someone who
more deeply understands the golang build process, and its
implementation within guix.

i think most of it is not very controversial, maybe except
the last commit.

i'm willing to reshape this under the guidance of someone who
has a better bird's eye view perspective on this all.

 guix/scripts/import.scm | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

M May 3, 2022, 4:21 p.m. UTC | #1
Attila Lendvai schreef op di 03-05-2022 om 13:16 [+0200]:
> [bug#55242] [PATCH 01/10] guix: import: Print the number of packages
> at the end.
> +                (format (current-warning-port)
> +                        (G_ "Imported ~a packages~%") count)))

What's the use case for counting the number of packages?

Why only for the Go importer, and why a warning? 

The 'info' macro from (guix diagnostics) seems to fit here (it also
uses 'current-warning-port', but only internally).

The plural form of 'packages' is incorrect when count=1.

Greetings,
Maxime
M May 3, 2022, 5:29 p.m. UTC | #2
Attila Lendvai schreef op di 03-05-2022 om 13:16 [+0200]:
> note that i only have a mediocre knowledge of the golang
> infrastructure, so this should be reviewed by someone who
> more deeply understands the golang build process, and its
> implementation within guix.
> 
> i think most of it is not very controversial, maybe except
> the last commit.

FWIW (given that I don't know the Go packaging system well), the last
patch (10/10) seems the least controversial to me.  I'm not seeing why
it would be controversial.

Greetings,
Maxime.
Attila Lendvai May 9, 2022, 12:37 p.m. UTC | #3
> > note that i only have a mediocre knowledge of the golang
> > infrastructure, so this should be reviewed by someone who
> > more deeply understands the golang build process, and its
> > implementation within guix.
> >
> > i think most of it is not very controversial, maybe except
> > the last commit.
>
>
> FWIW (given that I don't know the Go packaging system well), the last
> patch (10/10) seems the least controversial to me. I'm not seeing why
> it would be controversial.


because it introduces an in-source database of golang projects that
require special-casing (*module-path->import-path*).

i suspect that whatever info i'm storing in that db of exceptions is
probably available somewhere, somehow.

--
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Nobody in the world, nobody in history, has ever gotten their freedom by appealing to the moral sense of the people who were oppressing them.”
	— Assata Shakur (1947–)
M May 14, 2022, 7:09 a.m. UTC | #4
Hi,

I forgot that "guix build" already does some debug logging when passed
"--verbose" or "--debug".  Maybe "guix import ..." can gain a similar 
system.  I don't know if the internals are reusable though ...
(I'm not convinced that we need logging for "guix import" but I'm not
convinced of the contrary either, and there is already some kind of
debug logging system elsewhere.)

> here's how i was working on this codebase:
> 
> RUN=12
> clear && ./pre-inst-env guix import go -r --pin-versions  
> github.com/ethersphere/bee@v1.5.1 > >(tee -a
> ~/workspace/guix/importing/bee-run-${RUN}.scm) 2> >(tee -a
> ~/workspace/guix/importing/bee-run-${RUN}.log >&2) && beep
> 

FWIW, for these kind of things I do

  while ./pre-inst-env guix some-command; do echo 'Next'; done

and for your case (broken output without errors), maybe

  while true; do echo 'Next run' >> the-log-file && ./pre-inst-env guix some-command >> the-log-file; done

Greetings,
Maxime.
M May 14, 2022, 7:15 a.m. UTC | #5
> > That said, like "guix build" has a "--keep-going" option to
> temporarily
> > ignore to some degree packages that fail to build, maybe "guix
> import"
> > (when using --recursive) can have a "--keep-going" option.
> 
> 
> ok, i'll look into putting this feature behind a CLI argument, and
> generalize it to all importers. as time allows i'll get back to the
> rest of the questions/suggestions, too.

I'm not sure if I mentioned it previously, but it looks like some
patches are uncontroversial and usable mostly as-is (e.g. 04/10 without
the logging, 02/10, 05/10 with verifying types, 06/10 without the
overly-general false-if-exception and logging, 9/10, 10/10, 04/10). So
you could try the non-controversial bits first and then see if some
kind of consensus or such could be found for the remainder.

Greetings,
Maxime.
diff mbox series

Patch

diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index 40fa6759ae..891f558b97 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -127,10 +127,14 @@  (define-command (guix-import . args)
                             ('define-public _ ...)))
               (print expr))
              ((? list? expressions)
-              (for-each (lambda (expr)
-                          (print expr)
-                          (newline))
-                        expressions))
+              (let ((count 0))
+                (for-each (lambda (expr)
+                            (print expr)
+                            (set! count (1+ count))
+                            (newline))
+                          expressions)
+                (format (current-warning-port)
+                        (G_ "Imported ~a packages~%") count)))
              (x
               (leave (G_ "'~a' import failed~%") importer))))
          (let ((hint (string-closest importer importers #:threshold 3)))