GNU bug report logs - #38460
[PATCH 0/1] lint: Add '--load-path' option.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38460 in the body.
You can then email your comments to 38460 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH 0/1] lint: Add '--load-path' option.
Date: Mon,  2 Dec 2019 21:47:59 +0100

Dear,

Currently, when developing personal packages, "guix lint" needs the variable GUIX_PACKAGE_PATH which is not user friendly, IMO. The patch uses the common option '-L' to prepend a load path.

Other said,

  GUIX_PACKAGE_PATH=/path/to/my/package guix lint foo

becomes

  guix lint -L /path/to/my/package foo


All the best,
simon


zimoun (1):
  lint: Add '--load-path' option.

 guix/scripts/lint.scm |  8 ++++++++
 tests/guix-lint.sh    | 12 ++++++++++++
 2 files changed, 20 insertions(+)

-- 
2.23.0

Message #8 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: 38460 <at> debbugs.gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH 1/1] lint: Add '--load-path' option.
Date: Mon,  2 Dec 2019 22:01:27 +0100

* guix/scripts/lint.scm (%options): Add '--load-path' option.
* tests/guix-lint.sh: Test it.
---
 guix/scripts/lint.scm |  8 ++++++++
 tests/guix-lint.sh    | 12 ++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 1668d02992..8d08c484f5 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2017, 2018 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
+;;; Copyright © 2019 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -30,6 +31,7 @@
   #:use-module (guix lint)
   #:use-module (guix ui)
   #:use-module (guix scripts)
+  #:use-module (guix scripts build)
   #:use-module (gnu packages)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
@@ -94,6 +96,9 @@ run the checkers on all packages.\n"))
   -c, --checkers=CHECKER1,CHECKER2...
                          only run the specified checkers"))
   (display (G_ "
+  -L, --load-path=DIR    prepend DIR to the package module search path"))
+  (newline)
+  (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
   -l, --list-checkers    display the list of available lint checkers"))
@@ -128,6 +133,9 @@ run the checkers on all packages.\n"))
                               %local-checkers
                               (alist-delete 'checkers
                                             result))))
+        (find (lambda (option)
+                (member "load-path" (option-names option)))
+              %standard-build-options)
         (option '(#\h "help") #f #f
                 (lambda args
                   (show-help)
diff --git a/tests/guix-lint.sh b/tests/guix-lint.sh
index 7ddc7c265b..f07ccb4e1a 100644
--- a/tests/guix-lint.sh
+++ b/tests/guix-lint.sh
@@ -76,3 +76,15 @@ then true; else false; fi
 
 # Make sure specifying multiple packages works.
 guix lint -c inputs-should-be-native dummy dummy <at> 42 dummy
+
+
+# Use --load-path instead.
+unset GUIX_PACKAGE_PATH
+LOAD_PATH="$module_dir"
+
+out=`guix lint -L $LOAD_PATH -c synopsis,description dummy 2>&1`
+if [ `grep_warning "$out"` -ne 3 ]
+then false; else true; fi
+
+# Make sure specifying multiple packages works.
+guix lint -L $LOAD_PATH -c inputs-should-be-native dummy dummy <at> 42 dummy
-- 
2.23.0

Message #11 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 38460 <at> debbugs.gnu.org
Subject: Re: [bug#38460] [PATCH 1/1] lint: Add '--load-path' option.
Date: Wed, 04 Dec 2019 18:10:48 +0100

Hi!

zimoun <zimon.toutoune <at> gmail.com> skribis:

> * guix/scripts/lint.scm (%options): Add '--load-path' option.
> * tests/guix-lint.sh: Test it.

Awesome, I’ve been wanting that for a long time.  :-)

> +LOAD_PATH="$module_dir"
> +
> +out=`guix lint -L $LOAD_PATH -c synopsis,description dummy 2>&1`

Perhaps you don’t even need to define ‘LOAD_PATH’?

Could you add ‘-L’ to guix.texi, and then I guess we’re done!

Thanks,
Ludo’.

Message #14 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: 38460 <at> debbugs.gnu.org
Cc: zimoun <zimon.toutoune <at> gmail.com>
Subject: [PATCH v2] lint: Add '--load-path' option.
Date: Wed,  4 Dec 2019 19:06:31 +0100

* guix/scripts/lint.scm (%options): Add '--load-path' option.
* doc/guix.texi: Document it.
* tests/guix-lint.sh: Test it.
---
 doc/guix.texi         |  8 ++++++++
 guix/scripts/lint.scm |  8 ++++++++
 tests/guix-lint.sh    | 11 +++++++++++
 3 files changed, 27 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2da1ecd64c..e20d3fa722 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9676,6 +9676,14 @@ and exit.
 Only enable the checkers specified in a comma-separated list using the
 names returned by @code{--list-checkers}.
 
+@item --load-path=@var{directory}
+@itemx -L @var{directory}
+Add @var{directory} to the front of the package module search path
+(@pxref{Package Modules}).
+
+This allows users to define their own packages and make them visible to
+the command-line tools.
+
 @end table
 
 @node Invoking guix size
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 1668d02992..8d08c484f5 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2017, 2018 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2018, 2019 Arun Isaac <arunisaac <at> systemreboot.net>
+;;; Copyright © 2019 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -30,6 +31,7 @@
   #:use-module (guix lint)
   #:use-module (guix ui)
   #:use-module (guix scripts)
+  #:use-module (guix scripts build)
   #:use-module (gnu packages)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
@@ -94,6 +96,9 @@ run the checkers on all packages.\n"))
   -c, --checkers=CHECKER1,CHECKER2...
                          only run the specified checkers"))
   (display (G_ "
+  -L, --load-path=DIR    prepend DIR to the package module search path"))
+  (newline)
+  (display (G_ "
   -h, --help             display this help and exit"))
   (display (G_ "
   -l, --list-checkers    display the list of available lint checkers"))
@@ -128,6 +133,9 @@ run the checkers on all packages.\n"))
                               %local-checkers
                               (alist-delete 'checkers
                                             result))))
+        (find (lambda (option)
+                (member "load-path" (option-names option)))
+              %standard-build-options)
         (option '(#\h "help") #f #f
                 (lambda args
                   (show-help)
diff --git a/tests/guix-lint.sh b/tests/guix-lint.sh
index 7ddc7c265b..f0df1fda3a 100644
--- a/tests/guix-lint.sh
+++ b/tests/guix-lint.sh
@@ -76,3 +76,14 @@ then true; else false; fi
 
 # Make sure specifying multiple packages works.
 guix lint -c inputs-should-be-native dummy dummy <at> 42 dummy
+
+
+# Use --load-path instead.
+unset GUIX_PACKAGE_PATH
+
+out=`guix lint -L $module_dir -c synopsis,description dummy 2>&1`
+if [ `grep_warning "$out"` -ne 3 ]
+then false; else true; fi
+
+# Make sure specifying multiple packages works.
+guix lint -L $module_dir -c inputs-should-be-native dummy dummy <at> 42 dummy
-- 
2.23.0

Message #17 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 38460 <at> debbugs.gnu.org
Subject: Re: [bug#38460] [PATCH 1/1] lint: Add '--load-path' option.
Date: Wed, 4 Dec 2019 19:21:45 +0100

Hi,

On Wed, 4 Dec 2019 at 18:10, Ludovic Courtès <ludo <at> gnu.org> wrote:

> zimoun <zimon.toutoune <at> gmail.com> skribis:
>
> > * guix/scripts/lint.scm (%options): Add '--load-path' option.
> > * tests/guix-lint.sh: Test it.
>
> Awesome, I’ve been wanting that for a long time.  :-)

But I find this "hacky". Because it is copy/paste from other
--load-path option elsewhere (probably "guix show" :-)).
It should be refactored to have only one function and call it where
the command needs it.
I mean, the same way that 'show-build-options-help' or '(append
%transformation-options %standard-build-options)'  does in "guix
package".
Other said, %standard-build-options should be splited and go to say
misc.scm or common.scm or whatever -- I do not know enough the file
organization. :-)

Now, the same --load-path code at 3 different places. Bad practise... IMHO.

Plus, we will win more flexibility to write more commands. ;-)

What do you think?



> > +LOAD_PATH="$module_dir"
> > +
> > +out=`guix lint -L $LOAD_PATH -c synopsis,description dummy 2>&1`
>
> Perhaps you don’t even need to define ‘LOAD_PATH’?

Done.


> Could you add ‘-L’ to guix.texi, and then I guess we’re done!

I did.
My bad.


Cheers,
simon

Message #20 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 38460 <at> debbugs.gnu.org
Subject: Re: [bug#38460] [PATCH 1/1] lint: Add '--load-path' option.
Date: Sat, 07 Dec 2019 23:51:39 +0100

Hi!

zimoun <zimon.toutoune <at> gmail.com> skribis:

> On Wed, 4 Dec 2019 at 18:10, Ludovic Courtès <ludo <at> gnu.org> wrote:
>
>> zimoun <zimon.toutoune <at> gmail.com> skribis:
>>
>> > * guix/scripts/lint.scm (%options): Add '--load-path' option.
>> > * tests/guix-lint.sh: Test it.
>>
>> Awesome, I’ve been wanting that for a long time.  :-)
>
> But I find this "hacky". Because it is copy/paste from other
> --load-path option elsewhere (probably "guix show" :-)).

Well, technically speaking it’s not copy/pasted since the code you sent
uses ‘find’ to grab the option; plus, we’re talking about very few
lines, which is typically hard to factorize.

So I sympathize with the bad feeling of repetition, but I’m not quite
sure how this can be avoided in this case.

WDYT?

Thanks,
Ludo’.

Message #23 received at 38460 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 38460 <at> debbugs.gnu.org
Subject: Re: [bug#38460] [PATCH 1/1] lint: Add '--load-path' option.
Date: Sun, 8 Dec 2019 12:15:48 +0100

Hi Ludo,

On Sat, 7 Dec 2019 at 23:51, Ludovic Courtès <ludo <at> gnu.org> wrote:

> Well, technically speaking it’s not copy/pasted since the code you sent
> uses ‘find’ to grab the option; plus, we’re talking about very few
> lines, which is typically hard to factorize.

I agree. Even if I do not have a better solution to propose, I just
feel the current one is not optimal.

Does the same gettext entries G_ are duplicated? Do translators
translate 2 times (or more) the same string?


> So I sympathize with the bad feeling of repetition, but I’m not quite
> sure how this can be avoided in this case.
>
> WDYT?

Thank you for the explanations. I am not sure neither and need to fail
by myself to be convinced. ;-)
And currently, any hypothetical break should be reported by the test
suite. Hope so.


All the best,
simon

Message #28 received at 38460-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zimoun <zimon.toutoune <at> gmail.com>
Cc: 38460-done <at> debbugs.gnu.org
Subject: Re: [bug#38460] [PATCH 1/1] lint: Add '--load-path' option.
Date: Sun, 08 Dec 2019 15:17:44 +0100

Hi,

zimoun <zimon.toutoune <at> gmail.com> skribis:

> On Sat, 7 Dec 2019 at 23:51, Ludovic Courtès <ludo <at> gnu.org> wrote:
>
>> Well, technically speaking it’s not copy/pasted since the code you sent
>> uses ‘find’ to grab the option; plus, we’re talking about very few
>> lines, which is typically hard to factorize.
>
> I agree. Even if I do not have a better solution to propose, I just
> feel the current one is not optimal.
>
> Does the same gettext entries G_ are duplicated? Do translators
> translate 2 times (or more) the same string?

No, they’re deduplicated.

>> So I sympathize with the bad feeling of repetition, but I’m not quite
>> sure how this can be avoided in this case.
>>
>> WDYT?
>
> Thank you for the explanations. I am not sure neither and need to fail
> by myself to be convinced. ;-)
> And currently, any hypothetical break should be reported by the test
> suite. Hope so.

I think so.

I’ve applied it now, thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.