Hi Suhail,
I just submitted a new patch (v7) applying your suggestions and running
guix lint and guix style.
regarding your questions. I'll try to answer them below:

On Tue, 1 Oct 2024 at 03:07, Suhail Singh <suhailsingh247@gmail.com> wrote:

> 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 <omar.bassam88@gmail.com> 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?
>
>
This is expected as the jpm install command is meant to install janet
packages globally which would be impure.
To install janet packages to your local project directory, you need to add
the "-l" flag as follows:
jpm install -l sh

Alternatively you can also set the JPM_TREE environment variable to install
to a custom directory that you have access to.

Maybe in the future we can add a "janet-build-system" that will allow us to
add janet packages to the guix repository.


> 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.
>
>
These tests are not testing the installation of jpm, they are only testing
the "jpm install" command which will not work as I explained above.


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