unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#69661] [PATCH] gnu: Add redo-apenwarr
@ 2024-03-09  0:00 Massimo Zaniboni
       [not found] ` <3c9427f3-3857-4b54-9ad7-b8d970d734e7@protonmail.com>
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Massimo Zaniboni @ 2024-03-09  0:00 UTC (permalink / raw)
  To: 69661

Change-Id: Ied142a7dc3e9baf9babdeff046f350e647a7a5cc
---
  gnu/packages/build-tools.scm | 110 +++++++++++++++++++++++++++++++++++
  1 file changed, 110 insertions(+)

diff --git a/gnu/packages/build-tools.scm b/gnu/packages/build-tools.scm
index 15d88de..54de681 100644
--- a/gnu/packages/build-tools.scm
+++ b/gnu/packages/build-tools.scm
@@ -15,6 +15,7 @@
  ;;; Copyright © 2021 qblade <qblade@protonmail.com>
  ;;; Copyright © 2021, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
  ;;; Copyright © 2022, 2023 Juliana Sims <juli@incana.org>
+;;; Copyright © 2024 Massimo Zaniboni <mzan@dokmelody.org>
  ;;;
  ;;; This file is part of GNU Guix.
  ;;;
@@ -457,6 +458,115 @@ (define-public premake5
  scripted definition of a software project and outputs @file{Makefile}s or
  other lower-level build files.")))

+(define-public redo-apenwarr
+  (package
+    (name "redo-apenwarr")
+    (version "0.42d")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/apenwarr/redo")
+             (commit (string-append "redo-" version))))
+       (sha256
+        (base32 "0z78fmkb85k9hnpvh0pgi8cahljjgnr6j7mb614cykvpa3xsyl0p"))))
+
+    (build-system gnu-build-system)
+    (arguments
+     `(#:test-target "test"
+       #:parallel-build? #f
+       #:parallel-tests? #f
+       #:make-flags (list (string-append "PREFIX="
+                                         (assoc-ref %outputs "out"))
+                          "DESTDIR=/")
+       #:phases (modify-phases %standard-phases
+                  (delete 'configure)
+                  (add-before 'build 'patch-shell-scripts
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+
+                      (patch-shebang "minimal/do")
+
+                      ;; In Guix build phase there is no anymore a Git 
repo,
+                      ;; hence the Git tool cannot be anymore called.
+                      ;; So the content of the file is manually generated.
+                      (let* ((repo-version "0.42d")
+                             (repo-commit
+                              "7f00abc36be15f398fa3ecf9f4e5283509c34a00")
+                             (repo-date "2021-07-27 20:48:36 -0700")
+                             (repo-head (format #f
+                                         "(HEAD -> main, tag: redo-~a)"
+                                         repo-version)))
+
+                        (substitute* '("redo/version/gitvars.pre")
+                          (("\\$Format:%H\\$")
+                           repo-commit)
+                          (("\\$Format:%d\\$")
+                           repo-head)
+                          (("\\$Format:%ci\\$")
+                           repo-date)))
+
+                      ;; Redo scripts can inject shebangs headers to 
generated scripts.
+                      (substitute* '("bin/default.do"
+                                     "t/203-make/whichmake.do"
+                                     "redo/py.do"
+                                     "redoconf/link.od"
+                                     "redoconf/run.od"
+                                     "redoconf/link-shlib.od"
+                                     "redoconf/_compile.od"
+                                     "redoconf/compile.od"
+                                     "minimal/do")
+                        (("#!/bin/sh")
+                         (format #f "#!~a"
+                                 (which "sh"))))
+
+                      ;; Use `pwd' on the store.
+                      (substitute* '("t/all.do" "t/105-sympath/all.do"
+                                     "t/110-compile/hello.o.do" 
"minimal/do"
+                                     "minimal/do.test" "do")
+                        (("/bin/pwd")
+                         (which "pwd"))
+                        (("/bin/ls")
+                         (which "ls")))
+
+                      ;; Use `perl' on the store.
+                      (substitute* '("t/200-shell/nonshelltest.do")
+                        (("/usr/bin/env perl")
+                         (format #f "~a"
+                                 (which "perl"))))
+
+                      ;; Use `gcc' compiler, because Guix has no 
default `cc' compiler.
+                      (substitute* '("docs/cookbook/hello/hello.do"
+                                     "t/110-compile/LD.do"
+                                     "t/110-compile/CC.do"
+                                     "t/110-compile/yellow.o.do"
+                                     "t/111-example/CC.do"
+                                     "t/111-example/hello.do")
+                        (("^([ \t]*)cc " dummy starting-spaces)
+                         (string-append starting-spaces "gcc ")))
+
+                      (substitute* '("t/110-compile/all.do"
+                                     "t/111-example/all.do")
+                        ((" type cc ")
+                         " type gcc "))
+
+                      (substitute* '("docs/cookbook/c/flagtest.o.od")
+                        (("^CC=\"\\$CC\"")
+                         "CC=\"gcc\"")))))))
+
+    (inputs (list python-wrapper python-markdown python-beautifulsoup4))
+
+    (native-inputs
+     ;; Used for the tests.
+     (list which perl git gcc))
+
+    (synopsis
+     "Build tool where dependencies are part of the building instructions")
+    (description
+     "Redo-apenwarr is a build tool where each artifact is produced by 
a shell
+script having optional annotations for specifying its dependencies.")
+    (home-page "https://github.com/apenwarr/redo")
+    (license license:asl2.0)))
+
  (define-public scons
    (package
      (name "scons")

base-commit: d79c88e8809d2079452fd276bf4d17eb16636ff9
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH] gnu: Add redo-apenwarr
       [not found] ` <3c9427f3-3857-4b54-9ad7-b8d970d734e7@protonmail.com>
@ 2024-03-10 19:54   ` Skyler Ferris via Guix-patches via
  2024-03-12 15:39     ` Massimo Zaniboni
  0 siblings, 1 reply; 8+ messages in thread
From: Skyler Ferris via Guix-patches via @ 2024-03-10 19:54 UTC (permalink / raw)
  To: 69661

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

Hi Massimo,

Thank you for your submission. I am adding some specific notes about the package definition followed by some high-level notes about the overall package and the patch.

On 3/8/24 16:00, Massimo Zaniboni wrote:

> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url
> ["https://github.com/apenwarr/redo"](https://github.com/apenwarr/redo)
> )
> +             (commit (string-append "redo-" version))))
> +       (sha256
> +        (base32 "0z78fmkb85k9hnpvh0pgi8cahljjgnr6j7mb614cykvpa3xsyl0p"))))

`guix lint redo-apenwarr` reports an warning about the package name not being included in the source. This can be fixed by using the [file-name field in the origin](https://guix.gnu.org/manual/en/html_node/origin-Reference.html). This is helpful so that the store path clearly identifies the package it is related to. The README for this package mentions that there are other implementations of the redo build system. If an alternative implementation is added at some point in the future then store paths that simply identify themselves as "redo" will be ambiguous. Linting is one of the steps mentioned in the "[Submitting Patches](https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html)" section of the manual. If the output from this tool or any of the other steps is unclear please let me know and I'll do my best to help!

> +       #:parallel-build? #f
> +       #:parallel-tests? #f

Why is parallel building/testing disabled? Does this cause a build failure? If so, it would be preferable to find a way to fix it. If it cannot be fixed please add a comment explaining the problem.

> +       #:phases (modify-phases %standard-phases

Please make this modify-phases call into a [G-Expression](https://guix.gnu.org/manual/en/html_node/G_002dExpressions.html), as this is the updated convention and will become relevant to a later note. You will probably also want to change the arguments list from using a backtick to calling the `list` function.

> +                      (patch-shebang "minimal/do")

My understanding is that the patch-source-shebangs phase, which runs before build in gnu-build-system, should patch this file. Do you know why it was necessary to add this line?

> +
> +                      ;; In Guix build phase there is no anymore a Git
> repo,
> +                      ;; hence the Git tool cannot be anymore called.
> +                      ;; So the content of the file is manually generated.
> +                      (let* ((repo-version "0.42d")
> +                             (repo-commit
> +                              "7f00abc36be15f398fa3ecf9f4e5283509c34a00")
> +                             (repo-date "2021-07-27 20:48:36 -0700")
> +                             (repo-head (format #f
> +                                         "(HEAD -> main, tag: redo-~a)"
> +                                         repo-version)))
> +
> +                        (substitute* '("redo/version/gitvars.pre")
> +                          (("\\$Format:%H\\$")
> +                           repo-commit)
> +                          (("\\$Format:%d\\$")
> +                           repo-head)
> +                          (("\\$Format:%ci\\$")
> +                           repo-date)))

I see that git is included in native-inputs already, which should make it available in the build phase. If this is still a problem then I think there is something else we need to fix, rather than constructing the file manually.

> +                      ;; Redo scripts can inject shebangs headers to
> generated scripts.
> +                      (substitute* '("bin/default.do"
> +                                     "t/203-make/whichmake.do"
> +                                     "redo/py.do"
> +                                     "redoconf/link.od"
> +                                     "redoconf/run.od"
> +                                     "redoconf/link-shlib.od"
> +                                     "redoconf/_compile.od"
> +                                     "redoconf/compile.od"
> +                                     "minimal/do")
> +                        (("#!/bin/sh")
> +                         (format #f "#!~a"
> +                                 (which "sh"))))

In contrast to the patch-shebang call above, I think that this substitution is necessary because the shebang is in the contents of the script not the leading line (at least, this is the case for bin/default.do). However, the preferred way to locate binaries is with G-Expressions rather than looking them up dynamically. In this case I would expect the call to be `#$([file-append](https://guix.gnu.org/manual/en/html_node/G_002dExpressions.html#index-file_002dappend) bash "/bin/sh")`. There are some other places in the package where this also applies, but I will only annotate the ones that are meaningfully different.

> +                      ;; Use `perl' on the store.
> +                      (substitute* '("t/200-shell/nonshelltest.do")
> +                        (("/usr/bin/env perl")
> +                         (format #f "~a"
> +                                 (which "perl"))))

I don't think the format call is necessary here?

> +
> +                      ;; Use `gcc' compiler, because Guix has no
> default `cc' compiler.
> +                      (substitute* '("docs/cookbook/hello/hello.do"
> +                                     "t/110-compile/LD.do"
> +                                     "t/110-compile/CC.do"
> +                                     "t/110-compile/yellow.o.do"
> +                                     "t/111-example/CC.do"
> +                                     "t/111-example/hello.do")
> +                        (("^([ \t]*)cc " dummy starting-spaces)
> +                         (string-append starting-spaces "gcc ")))

While guix packages do not typically patch every single file to use absolute paths I think that it is better to use file-append since a substitution is happening here anyway.

> +
> +                      (substitute* '("docs/cookbook/c/flagtest.o.od")
> +                        (("^CC=\"\\$CC\"")
> +                         "CC=\"gcc\"")))))))

As above, file-append would be appropriate here. --- The output of the build works well. I was able to run the ["Hello, world!" example from the manual](https://redo.readthedocs.io/en/latest/cookbook/hello/) from a [container](https://guix.gnu.org/manual/en/html_node/Invoking-guix-shell.html#index-container) with only redo-apenwall, coreutils, and gcc-toolchain (coreutils might not have been necessary, but it was convenient for me and reasonable for a minimal container IMO). Additionally, building with [`--rounds=2`](https://guix.gnu.org/manual/en/html_node/Common-Build-Options.html) succeeded. Have you put any thought into what a redo-build-system in Guix might look like? Since it advertises itself as a make replacement I do not expect that packages using it would be naturally compatible with gnu-build-system.
I do not see any contents in the commit message other than the header line and Change-ID. This might be related to the problem I explain in the next paragraph but please make sure that the commit message follows the [Change Log format](https://www.gnu.org/prep/standards/standards.html#Change-Logs). You can look at previous commits in the log to see relevant examples, patches that add new packages typically have simple messages. As a final note, I want to address some problems I ran into with the patch format. I'm not sure if the source of the problem is on your machine or if it has something to do with the server but I have previously downloaded patches from this server and they applied cleanly so I think it might have to do with your email client. The problems I ran into include the following:

1. Unchanged lines were prefixed with 2 spaces instead of 1. This caused `git apply` to report an error that the unchanged text could not be found.
2. There were 3 erroneous line breaks. This caused `git apply` to report that the patch was corrupt.

You can see the version of the patch I received with `wget https://issues.guix.gnu.org/issue/69661/attachment/0`. I manually modified the patch file so that it applies cleanly in order to perform the above review. Would it be possible for you to try sending the next revision using `git send-email` as [described by the manual](https://guix.gnu.org/manual/en/html_node/Sending-a-Patch-Series.html)? Note that while the section is titled "Sending a Patch Series" it also applies to sending single patches.

[-- Attachment #2: Type: text/html, Size: 11813 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH v2] gnu: Add redo-apenwarr.
  2024-03-09  0:00 [bug#69661] [PATCH] gnu: Add redo-apenwarr Massimo Zaniboni
       [not found] ` <3c9427f3-3857-4b54-9ad7-b8d970d734e7@protonmail.com>
@ 2024-03-12 15:05 ` Massimo Zaniboni
  2024-05-16 21:01 ` [bug#69661] [PATCH vREVISION] " Massimo Zaniboni
  2 siblings, 0 replies; 8+ messages in thread
From: Massimo Zaniboni @ 2024-03-12 15:05 UTC (permalink / raw)
  To: 69661; +Cc: Massimo Zaniboni

* gnu/packages/build-tools.scm (redo-apenwarr): New variable.

Change-Id: Ied142a7dc3e9baf9babdeff046f350e647a7a5cc
---
 gnu/packages/build-tools.scm | 128 +++++++++++++++++++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/gnu/packages/build-tools.scm b/gnu/packages/build-tools.scm
index 15d88de..11dd337 100644
--- a/gnu/packages/build-tools.scm
+++ b/gnu/packages/build-tools.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2022, 2023 Juliana Sims <juli@incana.org>
+;;; Copyright © 2024 Massimo Zaniboni <mzan@dokmelody.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -457,6 +458,133 @@ (define-public premake5
 scripted definition of a software project and outputs @file{Makefile}s or
 other lower-level build files.")))
 
+(define-public redo-apenwarr
+  (let ((origin-url "https://github.com/apenwarr/redo")
+        (origin-date "2021-07-27 20:48:36 -0700")
+        (origin-version "0.42d")
+        (origin-commit "7f00abc36be15f398fa3ecf9f4e5283509c34a00"))
+    
+    (package
+      (name "redo-apenwarr")
+      (version origin-version)
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url origin-url)
+               (commit (string-append "redo-" version))))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32 "0z78fmkb85k9hnpvh0pgi8cahljjgnr6j7mb614cykvpa3xsyl0p"))))
+
+      (build-system gnu-build-system)
+      (arguments
+       (list
+        #:test-target "test"
+        #:make-flags #~(list (string-append "PREFIX="
+                                            #$output "/out") "DESTDIR=/")
+        #:phases #~(modify-phases %standard-phases
+                     (delete 'configure)
+                     (add-before 'build 'patch-shell-scripts
+                       (lambda* _
+                         
+                         ;; The building tool queries the Git repository,
+                         ;; for retrieving info about the commit hash and date,
+                         ;; but this information is not anymore present in the
+                         ;; source code downloaded from Guix.
+                         ;; So this information will be generated manually here,
+                         ;; using the data specified in the `origin-*' params.
+                         (substitute* '("redo/version/gitvars.pre")
+                           (("\\$Format:%H\\$")
+                            #$origin-commit)
+                           (("\\$Format:%d\\$")
+                            (format #f "(HEAD -> main, tag: redo-~a)"
+                                    #$origin-version))
+                           (("\\$Format:%ci\\$")
+                            #$origin-date))
+
+                         ;; redo-apenwarr can generate support scripts having
+                         ;; shebangs headers. We will patch these scripts
+                         ;; for using the `sh' in the store.
+                         (substitute* '("bin/default.do"
+                                        "t/203-make/whichmake.do"
+                                        "redo/py.do"
+                                        "redoconf/link.od"
+                                        "redoconf/run.od"
+                                        "redoconf/link-shlib.od"
+                                        "redoconf/_compile.od"
+                                        "redoconf/compile.od"
+                                        "minimal/do")
+                           (("#!/bin/sh")
+                            (string-append "#!"
+                                           #$(file-append bash "/bin/sh"))))
+
+                         ;; Use `pwd' on the store.
+                         (substitute* '("t/all.do" "t/105-sympath/all.do"
+                                        "t/110-compile/hello.o.do"
+                                        "minimal/do" "minimal/do.test" "do")
+                           (("/bin/pwd")
+                            #$(file-append coreutils "/bin/pwd"))
+                           (("/bin/ls")
+                            #$(file-append coreutils "/bin/ls")))
+
+                         ;; Use `perl' on the store.
+                         (substitute* '("t/200-shell/nonshelltest.do")
+                           (("/usr/bin/env perl")
+                            #$(file-append perl "/bin/perl")))
+
+                         ;; Use `gcc' compiler, because Guix has no default `cc' compiler.
+                         (substitute* '("docs/cookbook/hello/hello.do"
+                                        "t/110-compile/LD.do"
+                                        "t/110-compile/CC.do"
+                                        "t/110-compile/yellow.o.do"
+                                        "t/111-example/CC.do"
+                                        "t/111-example/hello.do")
+                           (("^([ \t]*)cc " dummy starting-spaces)
+                            (string-append starting-spaces
+                                           #$(file-append gcc "/bin/gcc") " ")))
+
+                         (substitute* '("t/110-compile/all.do"
+                                        "t/111-example/all.do")
+                           ((" type cc ")
+                            " type gcc "))
+
+                         (substitute* '("docs/cookbook/c/flagtest.o.od")
+                           (("^CC=\"\\$CC\"")
+                            (string-append "CC=\""
+                                           #$(file-append gcc "/bin/gcc") "\""))))))))
+
+      (inputs (list coreutils bash python-wrapper))
+      (native-inputs (list ;Used from the building script,
+                           ;; for checking the existences of coreutils on the path.
+                           which
+
+                           ;; Required for generating the documentation
+                           python-markdown
+                           python-beautifulsoup4
+
+                           ;; TODO it cannot generate some part of the the documentation,
+                           ;; because the disabled package python-mkdocs requires
+                           ;; the missing PyPI plugin python-mkdocs-exclude
+                           
+                           ;; Required only for the tests.
+                           perl
+                           git
+                           gcc))
+
+      (synopsis
+       "Build tool where dependencies are parts of the building instructions")
+      (description
+       "@code{redo-apenwarr} is a build tool where each target file is produced by
+a normal shell script, but with additional commands for specifying
+its dependencies.  In tools like @code{make}, dependencies are specified
+upfront (i.e. bottom-up), while in @code{redo-apenwarr} they are
+generated dynamically during build script execution (i.e. top-down).
+Rebuilds are incremental, i.e. only target files with changed dependencies
+will be rebuilt.")
+      (home-page "https://github.com/apenwarr/redo")
+      (license license:asl2.0))))
+
 (define-public scons
   (package
     (name "scons")

base-commit: d79c88e8809d2079452fd276bf4d17eb16636ff9
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH] gnu: Add redo-apenwarr
  2024-03-10 19:54   ` Skyler Ferris via Guix-patches via
@ 2024-03-12 15:39     ` Massimo Zaniboni
  2024-03-13  1:52       ` Skyler Ferris via Guix-patches via
  0 siblings, 1 reply; 8+ messages in thread
From: Massimo Zaniboni @ 2024-03-12 15:39 UTC (permalink / raw)
  To: Skyler Ferris, 69661

Hi Skyler,

many thanks for your detailed review!

1) I sent right now a PATCH for: testing git send-email; fixing all 
things in the review.

2) I don't consider this PATCH definitive, because: I can improve the 
way I generate documentation; I'm not using the package enough in 
production for being sure it is completely correct; a part of the 
package is probably not optimal. I will send a candidate PATCH after 
more testing and eventually your next review.

3) I will comment below to the questions of the past review...

> `guix lint redo-apenwarr` reports an warning about the package name not 
> being included in the source. This can be fixed by using the file-name 
> field in the origin 

DONE

> Linting is 
> one of the steps mentioned in the "Submitting Patches 
> <https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html>" 
> section of the manual. If the output from this tool or any of the other 
> steps is unclear please let me know and I'll do my best to help!

Now I obtain

```
$ make && ./pre-inst-env guix lint redo-apenwarr

make  all-recursive

^[]8;;file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm^[\gnu/packages/build-tools.scm:471:7^[]8;;^[\: 
warning: failed to fetch Git repository for redo-apenwarr
^[
]8;;file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm^[\gnu/packages/build-tools.scm:471:7^[]8;;^[\: 
redo-apenwarr@0.42d: updater 'generic-git' failed to find upstream 
releasesmake && ./pre-inst-env guix build -K redo-apenwarr
```

I have no idea how to fix them.

>> +       #:parallel-build? #f
>> +       #:parallel-tests? #f
> Why is parallel building/testing disabled? Does this cause a build 
> failure? If so, it would be preferable to find a way to fix it. If it 
> cannot be fixed please add a comment explaining the problem.

FIXED. Parallel builds and tests works correctly.

>> +       #:phases (modify-phases %standard-phases
> Please make this modify-phases call into a G-Expression 

DONE

>> +                      (patch-shebang "minimal/do")
> My understanding is that the patch-source-shebangs phase, which runs 
> before build in gnu-build-system, should patch this file. Do you know 
> why it was necessary to add this line?

FIXED. You were right: it is not necessary.

>> +
>> +                      ;; In Guix build phase there is no anymore a Git
>> repo,
>> +                      ;; hence the Git tool cannot be anymore called.
>> +                      ;; So the content of the file is manually generated.
>> +                      (let* ((repo-version "0.42d")
>> +                             (repo-commit
>> +                              "7f00abc36be15f398fa3ecf9f4e5283509c34a00")
>> +                             (repo-date "2021-07-27 20:48:36 -0700")
>> +                             (repo-head (format #f
>> +                                         "(HEAD -> main, tag: redo-~a)"
>> +                                         repo-version)))
>> +
>> +                        (substitute* '("redo/version/gitvars.pre")
>> +                          (("\\$Format:%H\\$")
>> +                           repo-commit)
>> +                          (("\\$Format:%d\\$")
>> +                           repo-head)
>> +                          (("\\$Format:%ci\\$")
>> +                           repo-date)))
> I see that git is included in native-inputs already, which should make 
> it available in the build phase. If this is still a problem then I think 
> there is something else we need to fix, rather than constructing the 
> file manually.

I improved the package code and the comments. Probably now it is more 
clear.

Put in other words: the redo-apenwarr installation script executes git 
commands for querying the git repo, and for deriving the date of the 
last commit. It uses this info for adding version/commit/date to the 
installed application.

Apparently, after the Guix git-fetch phase, there is no anymore this 
info, because there is no .git directory. So I generate this info 
"manually".

This "patch" is not elegant, and I'm open to suggestions about the 
correct way to handle this.

>> +                      ;; Redo scripts can inject shebangs headers to
>> generated scripts.
>> +                      (substitute* '("bin/default.do"
>> +                                     "t/203-make/whichmake.do"
>> +                                     "redo/py.do"
>> +                                     "redoconf/link.od"
>> +                                     "redoconf/run.od"
>> +                                     "redoconf/link-shlib.od"
>> +                                     "redoconf/_compile.od"
>> +                                     "redoconf/compile.od"
>> +                                     "minimal/do")
>> +                        (("#!/bin/sh")
>> +                         (format #f "#!~a"
>> +                                 (which "sh"))))
> In contrast to the patch-shebang call above, I think that this 
> substitution is necessary because the shebang is in the contents of the 
> script not the leading line (at least, this is the case for 
> bin/default.do). 

I double-checked all files. They are not header shebangs, but they are 
shebangs in the middle of the code, because they will be added to some 
generated files. So, I didn't changed this. It is correct.

> However, the preferred way to locate binaries is with 
> G-Expressions rather than looking them up dynamically. In this case I 
> would expect the call to be

DONE


>> +                      ;; Use `perl' on the store.
>> +                      (substitute* '("t/200-shell/nonshelltest.do")
>> +                        (("/usr/bin/env perl")
>> +                         (format #f "~a"
>> +                                 (which "perl"))))
> I don't think the format call is necessary here?

FIXED

>> +
>> +                      ;; Use `gcc' compiler, because Guix has no
>> default `cc' compiler.
>> +                      (substitute* '("docs/cookbook/hello/hello.do"
>> +                                     "t/110-compile/LD.do"
>> +                                     "t/110-compile/CC.do"
>> +                                     "t/110-compile/yellow.o.do"
>> +                                     "t/111-example/CC.do"
>> +                                     "t/111-example/hello.do")
>> +                        (("^([ \t]*)cc " dummy starting-spaces)
>> +                         (string-append starting-spaces "gcc ")))
> While guix packages do not typically patch every single file to use 
> absolute paths I think that it is better to use file-append since a 
> substitution is happening here anyway.

DONE

>> +
>> +                      (substitute* '("docs/cookbook/c/flagtest.o.od")
>> +                        (("^CC=\"\\$CC\"")
>> +                         "CC=\"gcc\"")))))))
> As above, file-append would be appropriate here.


DONE

--- The output of the
> build works well. I was able to run the "Hello, world!" example from the 
> manual <https://redo.readthedocs.io/en/latest/cookbook/hello/> from a 
> container 
> <https://guix.gnu.org/manual/en/html_node/Invoking-guix-shell.html#index-container> with only redo-apenwall, coreutils, and gcc-toolchain (coreutils might not have been necessary, but it was convenient for me and reasonable for a minimal container IMO). 

I added `coreutils` also to the inputs of the package.

TODO: I need to test better in production, for seeing if it is working 
all correctly, because despite it pass the tests, this tool interact 
with the rest of the profile/system during execution.

> Have you put any thought into what a redo-build-system in Guix might look like? Since it advertises itself as a make replacement I do not expect that packages using it would be naturally compatible with gnu-build-system.

I improved the description of the package. Previously I cited 
"artifacts", but this term is not 100% correct, because the tool can be 
used for producing many type of target files. It is more a 
build/automation tool for rebuilding the chain of dependencies. For 
example I'm using it for updating websites and so on.

So, it is not in competition with Guix, or other deterministic build 
systems. It is a mix between a normal scripts, but with annotations for 
having incremental updates. A corresponding Makefile would be a lot more 
verbose and hard to comprehend, because it will use a bottom-up paradigm 
in specifying dependencies.

Sadly, the main web-site of the tool is not describing it in a 
marketing-free language, so I had to create a description from scratch.

> I do not see any contents in the commit message other than the header 
> line and Change-ID. This might be related to the problem I explain in 
> the next paragraph but please make sure that the commit message follows 
> the Change Log format 

Yes probably.

> Would it be possible for you to try sending the next 
> revision using `git send-email` as described by the manual 
> <https://guix.gnu.org/manual/en/html_node/Sending-a-Patch-Series.html>? 
> Note that while the section is titled "Sending a Patch Series" it also 
> applies to sending single patches.

DONE. I hope this time it will be better. But, it is the first time I'm 
using it.

Regards,
Massimo




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH] gnu: Add redo-apenwarr
  2024-03-12 15:39     ` Massimo Zaniboni
@ 2024-03-13  1:52       ` Skyler Ferris via Guix-patches via
  2024-03-15  2:19         ` Skyler Ferris via Guix-patches via
  0 siblings, 1 reply; 8+ messages in thread
From: Skyler Ferris via Guix-patches via @ 2024-03-13  1:52 UTC (permalink / raw)
  To: Massimo Zaniboni, 69661

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

On 3/12/24 08:39, Massimo Zaniboni wrote:

> 2) I don't consider this PATCH definitive, because: I can improve the
> way I generate documentation; I'm not using the package enough in
> production for being sure it is completely correct; a part of the
> package is probably not optimal. I will send a candidate PATCH after
> more testing and eventually your next review.

Ok, I'll add the "moreinfo" tag to this issue until you indicate that it is ready for merging.

> Now I obtain
>
> ```
> $ make && ./pre-inst-env guix lint redo-apenwarr
>
> make  all-recursive
>
> ]8;;
> [file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm\gnu/packages/build-tools.scm:471:7](file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm^[\gnu/packages/build-tools.scm:471:7^[)
> ]8;;\:
> warning: failed to fetch Git repository for redo-apenwarr
>
> ]8;;
> [file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm\gnu/packages/build-tools.scm:471:7](file://think/home/mzan/tmp/guix/guix-cloned-channel/gnu/packages/build-tools.scm^[\gnu/packages/build-tools.scm:471:7^[)
> ]8;;\:
> redo-apenwarr@0.42d
> : updater 'generic-git' failed to find upstream
> releasesmake && ./pre-inst-env guix build -K redo-apenwarr
> ```
>
> I have no idea how to fix them.

I'm not seeing this on my machine, just a couple of warnings about trailing whitespaces. Since it failed to fetch a resource over the network I'm wondering if this was a temporary error that was preventing your machine from reaching the server? Let me know if you still get this error after successfully cloning the repository from the same machine (manually, with `git clone`).

> I improved the package code and the comments. Probably now it is more
> clear.
>
> Put in other words: the redo-apenwarr installation script executes git
> commands for querying the git repo, and for deriving the date of the
> last commit. It uses this info for adding version/commit/date to the
> installed application.
>
> Apparently, after the Guix git-fetch phase, there is no anymore this
> info, because there is no .git directory. So I generate this info
> "manually".
>
> This "patch" is not elegant, and I'm open to suggestions about the
> correct way to handle this.

This is very strange. I have used git commands in build phases before and there was no issue. I was using the same git-fetch origin type. The git folder *should* be there but when I remove the relevant snippet from the build phase and run with --keep-failed it is not. I'll look at this more closely when I have some time.

>> Would it be possible for you to try sending the next
>> revision using `git send-email` as described by the manual
>> [<https://guix.gnu.org/manual/en/html_node/Sending-a-Patch-Series.html>](https://guix.gnu.org/manual/en/html_node/Sending-a-Patch-Series.html)
>> ?
>> Note that while the section is titled "Sending a Patch Series" it also
>> applies to sending single patches.
>
> DONE. I hope this time it will be better. But, it is the first time I'm
> using it.

Looks like it worked! I was able to apply the patch locally without modification and I see your complete commit message now.

There are also a couple of small things about the new version of the patch that should be changed. In the PREFIX value for makeflags, "/out" should not be appended to #$output. This causes binaries to be installed to "out/bin" instead of "bin", so they are not actually in the path when users add the package to their profile. Also, since the build phase no longer uses the arguments given it can be a plain lambda instead of lambda*. You can include these changes when you send the next version that improves the documentation generation, no need to send an intermediate patch for these small changes.

[-- Attachment #2: Type: text/html, Size: 5349 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH] gnu: Add redo-apenwarr
  2024-03-13  1:52       ` Skyler Ferris via Guix-patches via
@ 2024-03-15  2:19         ` Skyler Ferris via Guix-patches via
  0 siblings, 0 replies; 8+ messages in thread
From: Skyler Ferris via Guix-patches via @ 2024-03-15  2:19 UTC (permalink / raw)
  To: Massimo Zaniboni, 69661

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

On 3/12/24 18:52, Skyler Ferris wrote:

>> This is very strange. I have used git commands in build phases before and there was no issue. I was using the same git-fetch origin type. The git folder *should* be there but when I remove the relevant snippet from the build phase and run with --keep-failed it is not. I'll look at this more closely when I have some time.

So, it turns out the reason that it worked for me previously is that I only used `git apply`, which is happy to apply patches even if they are not in a git repository, because the patch file itself has all the information it needs. So I guess this actually is expected.

I don't think that the manual substitution is necessarily a problem. But if you want to avoid it, perhaps using the [tagged release](https://github.com/apenwarr/redo/releases/tag/redo-0.42d) would work better? There is a comment in redo/version/gitvars.do that says that tarballs should have the correct data baked in.

[-- Attachment #2: Type: text/html, Size: 1621 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH vREVISION] gnu: Add redo-apenwarr.
  2024-03-09  0:00 [bug#69661] [PATCH] gnu: Add redo-apenwarr Massimo Zaniboni
       [not found] ` <3c9427f3-3857-4b54-9ad7-b8d970d734e7@protonmail.com>
  2024-03-12 15:05 ` [bug#69661] [PATCH v2] " Massimo Zaniboni
@ 2024-05-16 21:01 ` Massimo Zaniboni
  2024-05-16 21:11   ` Massimo Zaniboni
  2 siblings, 1 reply; 8+ messages in thread
From: Massimo Zaniboni @ 2024-05-16 21:01 UTC (permalink / raw)
  To: 69661; +Cc: Massimo Zaniboni

* gnu/packages/build-tools.scm (redo-apenwarr): New variable.

Change-Id: Ied142a7dc3e9baf9babdeff046f350e647a7a5cc
---
 gnu/packages/build-tools.scm | 115 +++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/gnu/packages/build-tools.scm b/gnu/packages/build-tools.scm
index daaf450e70..5f575c8aff 100644
--- a/gnu/packages/build-tools.scm
+++ b/gnu/packages/build-tools.scm
@@ -15,6 +15,7 @@
 ;;; Copyright © 2021 qblade <qblade@protonmail.com>
 ;;; Copyright © 2021, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2022, 2023 Juliana Sims <juli@incana.org>
+;;; Copyright © 2024 Massimo Zaniboni <mzan@dokmelody.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -464,6 +465,120 @@ (define-public premake5
 scripted definition of a software project and outputs @file{Makefile}s or
 other lower-level build files.")))
 
+(define-public redo-apenwarr
+  (let ((origin-url "https://github.com/apenwarr/redo")
+        (origin-date "2021-07-27 20:48:36 -0700")
+        (origin-version "0.42d")
+        (origin-commit "7f00abc36be15f398fa3ecf9f4e5283509c34a00"))
+    (package
+      (name "redo-apenwarr")
+      (version origin-version)
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url origin-url)
+               (commit (string-append "redo-" version))))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32 "0z78fmkb85k9hnpvh0pgi8cahljjgnr6j7mb614cykvpa3xsyl0p"))))
+      (build-system gnu-build-system)
+      (arguments
+       (list
+        #:test-target "test"
+        #:make-flags #~(list (string-append "DESTDIR="
+                                            #$output) "PREFIX=/")
+        #:phases #~(modify-phases %standard-phases
+                     (delete 'configure)
+                     (add-before 'build 'patch-shell-scripts
+                       (lambda _
+                         ;; The building tool queries the Git repository,
+                         ;; for retrieving info about the commit hash and date,
+                         ;; but this information is not anymore present in the
+                         ;; source code downloaded from Guix.
+                         ;; So this information will be generated manually here,
+                         ;; using the data specified in the `origin-*' params.
+                         (substitute* '("redo/version/gitvars.pre")
+                           (("\\$Format:%H\\$")
+                            #$origin-commit)
+                           (("\\$Format:%d\\$")
+                            (format #f "(HEAD -> main, tag: redo-~a)"
+                                    #$origin-version))
+                           (("\\$Format:%ci\\$")
+                            #$origin-date))
+
+                         ;; redo-apenwarr can generate support scripts having
+                         ;; shebangs headers. We will patch these scripts
+                         ;; for using the `sh' in the store.
+                         (substitute* '("bin/default.do"
+                                        "t/203-make/whichmake.do"
+                                        "redo/py.do"
+                                        "redoconf/link.od"
+                                        "redoconf/run.od"
+                                        "redoconf/link-shlib.od"
+                                        "redoconf/_compile.od"
+                                        "redoconf/compile.od"
+                                        "minimal/do")
+                           (("#!/bin/sh")
+                            (string-append "#!"
+                                           #$(file-append bash "/bin/sh"))))
+
+                         ;; Use `pwd' on the store.
+                         (substitute* '("t/all.do" "t/105-sympath/all.do"
+                                        "t/110-compile/hello.o.do"
+                                        "minimal/do" "minimal/do.test" "do")
+                           (("/bin/pwd")
+                            #$(file-append coreutils "/bin/pwd"))
+                           (("/bin/ls")
+                            #$(file-append coreutils "/bin/ls")))
+
+                         ;; Use `perl' on the store.
+                         (substitute* '("t/200-shell/nonshelltest.do")
+                           (("/usr/bin/env perl")
+                            #$(file-append perl "/bin/perl")))
+
+                         ;; Use `gcc' compiler, because Guix has no default `cc' compiler.
+                         (substitute* '("docs/cookbook/hello/hello.do"
+                                        "t/110-compile/LD.do"
+                                        "t/110-compile/CC.do"
+                                        "t/110-compile/yellow.o.do"
+                                        "t/111-example/CC.do"
+                                        "t/111-example/hello.do")
+                           (("^([ \t]*)cc " dummy starting-spaces)
+                            (string-append starting-spaces
+                                           #$(file-append gcc "/bin/gcc") " ")))
+
+                         (substitute* '("t/110-compile/all.do"
+                                        "t/111-example/all.do")
+                           ((" type cc ")
+                            " type gcc "))
+
+                         (substitute* '("docs/cookbook/c/flagtest.o.od")
+                           (("^CC=\"\\$CC\"")
+                            (string-append "CC=\""
+                                           #$(file-append gcc "/bin/gcc") "\""))))))))
+      (inputs (list coreutils bash python-wrapper))
+      (native-inputs (list ;Used from the building script,
+                           ;; for checking the existences of coreutils on the path.
+                           which
+
+                           ;; Required only for the tests.
+                           perl
+                           git
+                           gcc))
+      (synopsis
+       "Build tool where dependencies are parts of the building instructions")
+      (description
+       "@code{redo-apenwarr} is a build tool where each target file is produced by
+a normal shell script, but with additional commands for specifying
+its dependencies.  In tools like @code{make}, dependencies are specified
+upfront (i.e. bottom-up), while in @code{redo-apenwarr} they are
+generated dynamically during build script execution (i.e. top-down).
+Rebuilds are incremental, i.e. only target files with changed dependencies
+will be rebuilt.")
+      (home-page "https://github.com/apenwarr/redo")
+      (license license:asl2.0))))
+
 (define-public scons
   (package
     (name "scons")

base-commit: 5a624adfd7b14c3717237d137bd0766c77f0f570
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#69661] [PATCH vREVISION] gnu: Add redo-apenwarr.
  2024-05-16 21:01 ` [bug#69661] [PATCH vREVISION] " Massimo Zaniboni
@ 2024-05-16 21:11   ` Massimo Zaniboni
  0 siblings, 0 replies; 8+ messages in thread
From: Massimo Zaniboni @ 2024-05-16 21:11 UTC (permalink / raw)
  To: 69661

I'm using the package, so this time it should be correct.

I was not able to generate the documentation, because the required 
python-mkdocs plugin at https://github.com/apenwarr/mkdocs-exclude is 
not anymore compatible with the last versions of python-mkdocs. It is 
probably a simple fix, but I have no time, and documentation can be 
found also online.

Maybe, I will add documentation in future, if I continue to use the package.

Best regards,
Massimo





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-05-16 21:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-09  0:00 [bug#69661] [PATCH] gnu: Add redo-apenwarr Massimo Zaniboni
     [not found] ` <3c9427f3-3857-4b54-9ad7-b8d970d734e7@protonmail.com>
2024-03-10 19:54   ` Skyler Ferris via Guix-patches via
2024-03-12 15:39     ` Massimo Zaniboni
2024-03-13  1:52       ` Skyler Ferris via Guix-patches via
2024-03-15  2:19         ` Skyler Ferris via Guix-patches via
2024-03-12 15:05 ` [bug#69661] [PATCH v2] " Massimo Zaniboni
2024-05-16 21:01 ` [bug#69661] [PATCH vREVISION] " Massimo Zaniboni
2024-05-16 21:11   ` Massimo Zaniboni

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).