Subject: Re: [bug#72925] [PATCH v3] adding jpm package

From: Suhail Singh <[email-masked]>
Date: Mon, 30 Sep 2024 20:06:48 -0400

Omar, thank you for sending a revised patch.  I have a few comments
relating to style and one unanswered question from our last exchange.

> Subject: [bug#72925] [PATCH v3] adding jpm package

In v4, could you please update the commit message to conform to the
ChangeLog format as noted in
<https://guix.gnu.org/manual/devel/en/html_node/Submitting-Patches.html>.
Please see
<https://www.gnu.org/prep/standards/html_node/Change-Logs.html#Change-Logs>
for additional details.  If you're using magit,
`magit-generate-changelog' can help with this.

In your case the commit will probably look something like:
#+begin_quote
  gnu: Add jpm.

  * gnu/packages/lisp.scm (jpm): New variable.
#+end_quote

Omar Bassam <[email-masked]> writes:

> +(define-public jpm
> +  (package
> +    (name "jpm")
> +    (version "1.1.0")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/janet-lang/jpm.git")
> +                    (commit (string-append "v" version))))
> +              (file-name (git-file-name name version))
> +              (sha256 (base32 "05rdxigmiy7vf93s16a8n2029lq33073jccz1rjl4iisxj6piw4l"))))

There are no build errors with this, however, it's not clear how to
verify that the runtime behaviour of jpm is as expected.  After
installing janet and jpm in a guix profile, running a command such as:

#+begin_src sh
  jpm install sh
#+end_src

Results in the following:

#+begin_example
  $> jpm install sh
  error: Read-only file system: /gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/.cache
    in os/mkdir [src/core/os.c] on line 1981
    in download-bundle [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/pm.janet] on line 200, column 3
    in bundle-install [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/pm.janet] on line 217, column 13
    in resolve-bundle-name [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/pm.janet] on line 118, column 20
    in resolve-bundle [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/pm.janet] on line 148, column 9
    in bundle-install [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/pm.janet] on line 216, column 4
    in install [/gnu/store/ffmis4y6rld42biqx5lq4nvsjp0bqiq6-jpm-1.1.0/lib/janet/jpm/commands.janet] (tail call) on line 190, column 20
    in run-main [boot.janet] on line 4432, column 16
    in cli-main [boot.janet] on line 4613, column 17
#+end_example

Could you please share an example code snippet which can be used to
verify correctness of the installation?

Additionally, it seems that the jpm repository comes with a test
(./test/installtest.janet and ./testinstall).  However, it doesn't seem
like we're running it during the build.  Could you please share the
reasons why?  If possible, we should enable and run these tests.

> +    (build-system copy-build-system)
> +    (arguments
> +     (list
> +      #:phases #~(modify-phases %standard-phases
> +		   (add-after 'unpack 'fix-prefix-path
> +		     (lambda _
> +		       (substitute* "configs/linux_config.janet"
> +			 (("/usr/local") #$output))
> +		       (setenv "PREFIX" #$output)))

> +		   (replace 'install
> +		     (lambda _ 

V3 doesn't cleanly apply due to whitespace issues on this (^) line.
Please fix.

On a related note, in case you're not aware, please observe all the
steps listed in
<https://guix.gnu.org/manual/devel/en/html_node/Submitting-Patches.html>.
Steps 3 and 4 recommend invoking guix lint and guix style which, unless
I'm mistaken, would've caught this issue.

-- 
Suhail