Re: [Outreachy] Feedback on 'guix git log' subcommand

[zimoun <zimon.toutoune@gmail.com>]

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

Hi Magali,

On Thu, 28 Jan 2021 at 00:53, Magali <magalilemes00@gmail.com> wrote:

> On a side note, I've been blogging about it at
> <https://magalilemes.gitlab.io/blog>.

Nice readings! :-)

> Below are a few examples of the options currently available:
>
> ./pre-inst-env guix git log --format=medium
> ./pre-inst-env guix git log --oneline
> ./pre-inst-env guix git log --channel-cache-path
> ./pre-inst-env guix git log --channel-cache-path=guix

Cool!

An example, the recent update of setuptools breaks python2-setuptools.

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix git log --oneline \
>   | grep 'python-setuptools:' | grep Update
b3ca2b4 gnu: python-setuptools: Update to 40.8.0.
d0205df gnu: python-setuptools: Update to 40.0.0.
62a9a23 gnu: python-setuptools: Update to 18.3.1.
d3d656c gnu: python-setuptools: Update to 12.1.
d660f7b gnu: python-setuptools: Update to 31.0.0.
e39d493 gnu: python-setuptools: Update to 41.0.1.
3142dac gnu: python-setuptools: Update to 52.0.0.
--8<---------------cut here---------------end--------------->8---

Ah, the order seems weird, “git log” says:

--8<---------------cut here---------------start------------->8---
3142daccbe gnu: python-setuptools: Update to 52.0.0.
e39d4933d1 gnu: python-setuptools: Update to 41.0.1.
b3ca2b45d1 gnu: python-setuptools: Update to 40.8.0.
d0205dfd92 gnu: python-setuptools: Update to 40.0.0.
d660f7be6d gnu: python-setuptools: Update to 31.0.0.
62a9a23bf9 gnu: python-setuptools: Update to 18.3.1.
d3d656c5fb gnu: python-setuptools: Update to 12.1.
--8<---------------cut here---------------end--------------->8---

Well, somehow it is your question about which order. :-)

Doing that, I am thinking of two nice features:

  1. get the parent commits
  2. show a specific commit

For example,

  guix git log --parents=3142daccbe # => 631e1f33
  guix git log --show=3142daccbe

so then I can feed “guix time-machine” with 631e1f33 to have
’python2-setuptools’ before it breaks.  I am not sure to what “--show”
would do.

WDYT?

Hum, ’--grep’ seems missing.  I do not remember, is it not working?
Even slowly?


As discussed, “guix-git-log” is not defined as ’define-command’ because
it should not appear on “guix help”.  However, it should appear with
“guix git --help”.

Well, my idea was something along this attached patch,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: small.patch --]
[-- Type: text/x-diff, Size: 1821 bytes --]

diff --git a/guix/scripts/git.scm b/guix/scripts/git.scm
index 8fcd0ccca8..3b141a622d 100644
--- a/guix/scripts/git.scm
+++ b/guix/scripts/git.scm
@@ -18,18 +18,26 @@
 
 (define-module (guix scripts git)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 format)
   #:use-module (guix ui)
   #:use-module (guix scripts)
+  #:use-module (srfi srfi-1)
   #:export (guix-git))
 
+(define %sub-commands
+  `(("authenticate" . "verify commit signatures and authorizations")
+    ("log". "show Git commit history")))
+
 (define (show-help)
   (display (G_ "Usage: guix git COMMAND ARGS...
 Operate on Git repositories.\n"))
   (newline)
   (display (G_ "The valid values for ACTION are:\n"))
   (newline)
-  (display (G_ "\
-   authenticate    verify commit signatures and authorizations\n"))
+  (for-each (match-lambda
+              ((name . help)
+               (format #t "~13a ~a\n" name help)))
+            %sub-commands)
   (newline)
   (display (G_ "
   -h, --help             display this help and exit"))
@@ -38,8 +46,6 @@ Operate on Git repositories.\n"))
   (newline)
   (show-bug-report-information))
 
-(define %sub-commands '("authenticate" "log"))
-
 (define (resolve-sub-command name)
   (let ((module (resolve-interface
                  `(guix scripts git ,(string->symbol name))))
@@ -61,7 +67,6 @@ Operate on Git repositories.\n"))
       ((or ("-V") ("--version"))
        (show-version-and-exit "guix git"))
       ((sub-command args ...)
-       (if (member sub-command %sub-commands)
+       (if (find (lambda (s) (string=? (first s) sub-command)) %sub-commands)
            (apply (resolve-sub-command sub-command) args)
-           (format (current-error-port)
-                   (G_ "guix git: invalid sub-command~%")))))))
+           (leave (G_ "~a: invalid sub-command~%") sub-command))))))

[-- Attachment #3: Type: text/plain, Size: 441 bytes --]


but “guix system” hard code all the subcommands.  So, maybe hardcoding
the “log” subcommand is the way to do; with translation in mind, I
guess.



> Another thing is that the command is a bit slower than 'git log' itself.
> Thoughts on how that could be improved?

I will provide more on a separate email.  And review more in details the
code. :-)


Thanks for working on that.  That’s cool! :-)

Cheers,
simon