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