[bug#60013] [PATCH] gnu: Add clight

[bug#60013] [PATCH] gnu: Add clight

[Florian]

[bug#60013] [PATCH] gnu: Add clight

[zimoun]

Hi,

On Mon, 12 Dec 2022 at 16:53, Florian <florhizome@posteo.net> wrote:

It seems empty. :-)


Cheers,
simon

[bug#60013] [PATCH] gnu: Add clight

[Tobias Geerinckx-Rice via Guix-patches via]

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

zimoun 写道：
> It seems empty. :-)

They were probably waiting for a bug number and it (or their 
patches) haven't arrived yet.

Ah, debbugs,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[florhizome]

From: florhizome <florhizome@posteo.net>

zimoun: yes I usually send an empty mail before a patchset wit multiple patches

* gnu/packages/wm.scm (libmodule): New variable.
---
 gnu/packages/c.scm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/gnu/packages/c.scm b/gnu/packages/c.scm
index fb876eeb71..3afdaa995a 100644
--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm
@@ -1300,6 +1300,28 @@ (define-public libdispatch
 will take care of dispatching tasks to available cores.")
     (license license:asl2.0)))
 
+
+(define-public libmodule
+  (package
+    (name "libmodule")
+    (version "5.0.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/FedeDP/libmodule")
+             (commit version)))
+       (sha256
+        (base32 "1j48gkj6zlifrmx9qh37ijmqjvdvgmpvxah7j2qlrhdxcc6n4i62"))))
+    (build-system cmake-build-system)
+    (native-inputs (list pkg-config))
+    (arguments '(#:tests? #f)) ;tests not found
+    (home-page "https://github.com/FedeDP/libmodule")
+    (synopsis "Simple and elegant implementation of an actor library in C")
+    (description "Libmodule offers a small and simple C implementation of an
+ actor library that aims to let developers easily create modular C projects.")
+    (license license:expat)))
+
 (define-public utf8-h
   ;; The latest tag is used as there is no release.
   (let ((commit "500d4ea9f4c3449e5243c088d8af8700f7189734")

base-commit: 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
prerequisite-patch-id: 36ae907c0ae2cbc001f774c0514ab217855270c2
prerequisite-patch-id: f85768858f3a8b1ef44b6e355dcd7f9254c07d24
prerequisite-patch-id: 2c99b804c1a929fc9d74b4c3d92263cbd296f785
prerequisite-patch-id: 2525aea715c2eb5be5f61e2e14296a36898413ca
prerequisite-patch-id: f7afbf36e2776eced1e69090ec127a40456efca4
prerequisite-patch-id: ba73cf06ab2610e36d03df1f6b6a1c4b8f271cda
prerequisite-patch-id: 9ac4b32603488e776af63831ff13406c649a8686
prerequisite-patch-id: 1cec3e6ed7a13938c53c24f816056d98b2b005c7
prerequisite-patch-id: 011ca6e0eaf4ea7077787d7030f193836c30aa82
prerequisite-patch-id: 4243cf3f6370445e6d583f03231e87921f541cc2
prerequisite-patch-id: 6c88bfec7b2fa53face9523329de4a9102149772
prerequisite-patch-id: d1bed6e8da46c3d6f4017546f7a00c001e814fe1
prerequisite-patch-id: 1ff1bcd9503a2b38ca7cc59ebdcebb45f970cf1a
prerequisite-patch-id: 4aa5ec94c128f5b6b6b6709c25baa49fffa571f6
prerequisite-patch-id: cb8f4f83e84db03ffa3c6be6c6b0df2ef0b59422
prerequisite-patch-id: 709b4522fd225f3c28bb2721e5dd9a20014ce131
-- 
2.38.1

[bug#60013] [PATCH 2/3] gnu: Add clightd

[florhizome]

From: florhizome <florhizome@posteo.net>

* gnu/packages/wm.scm (clightd): New variable.
---
 gnu/packages/wm.scm | 69 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm
index 86573658f4..919bcc9400 100644
--- a/gnu/packages/wm.scm
+++ b/gnu/packages/wm.scm
@@ -61,6 +61,7 @@
 ;;; Copyright © 2022 ( <paren@disroot.org>
 ;;; Copyright © 2022 zamfofex <zamfofex@twdb.moe>
 ;;;
+;;; Copyright © 2022 florhizome <florhizome@posteo.net>
 ;;; This file is part of GNU Guix.
 ;;;
 ;;; GNU Guix is free software; you can redistribute it and/or modify it
@@ -102,6 +103,7 @@ (define-module (gnu packages wm)
   #:use-module (gnu packages calendar)
   #:use-module (gnu packages check)
   #:use-module (gnu packages cpp)
+  #:use-module (gnu packages c)
   #:use-module (gnu packages datastructures)
   #:use-module (gnu packages docbook)
   #:use-module (gnu packages documentation)
@@ -114,6 +116,7 @@ (define-module (gnu packages wm)
   #:use-module (gnu packages gperf)
   #:use-module (gnu packages gtk)
   #:use-module (gnu packages gnome)
+  #:use-module (gnu packages hardware)
   #:use-module (gnu packages haskell-check)
   #:use-module (gnu packages haskell-web)
   #:use-module (gnu packages haskell-xyz)
@@ -121,6 +124,7 @@ (define-module (gnu packages wm)
   #:use-module (gnu packages imagemagick)
   #:use-module (gnu packages libevent)
   #:use-module (gnu packages libffi)
+  #:use-module (gnu packages libusb)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages lisp-check)
   #:use-module (gnu packages lisp-xyz)
@@ -132,6 +136,7 @@ (define-module (gnu packages wm)
   #:use-module (gnu packages mpd)
   #:use-module (gnu packages pcre)
   #:use-module (gnu packages perl)
+  #:use-module (gnu packages polkit)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages pretty-print)
   #:use-module (gnu packages pulseaudio)
@@ -188,6 +193,70 @@ (define-public bspwm
 the leaves of a full binary tree.")
     (license license:bsd-2)))
 
+
+(define-public clightd
+  (package
+    (name "clightd")
+    (version "5.7")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/FedeDP/clightd")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "1b6hc98vk0nm06plld4rgk70f825g8kakcxrv2prsxxvkvlqrmp1"))))
+    (build-system cmake-build-system)
+    (native-inputs (list pkg-config libmodule))
+    (inputs (list elogind
+                  eudev
+                  dbus
+                  ddcutil
+                  libdrm
+                  libjpeg-turbo
+                  libmodule
+                  libxrandr
+                  libdrm
+                  libusb
+                  libx11
+                  libxext
+                  polkit-duktape
+                  wayland
+                  wayland-protocols))
+    (arguments
+     (list
+      #:tests? #f  ;;;tests not found
+      #:configure-flags #~(list "-DENABLE_GAMMA=1"
+                                "-DENABLE_DPMS=1"
+                                "-DENABLE_DDC=1"
+                                "-DENABLE_SCREEN=1"
+                                "-DENABLE_YOCTOLIGHT=1"
+                                (string-append "-DMODULE_LOAD_DIR=" #$output
+                                               "/etc/modprobe.d")
+                                (string-append "-DDBUS_CONFIG_DIR=" #$output
+                                               "/share/dbus-1/system.d"))
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after
+              'patch-source-shebangs 'patch-target-dirs
+            (lambda _
+              (substitute* "./CMakeLists.txt"
+                (("DESTINATION \\$\\{SYSTEM_BUS_DIR\\}")
+                 (string-append "DESTINATION "
+                                #$output "/share/dbus-1/system-services"))
+                (("DESTINATION \\$\\{POLKIT_ACTION_DIR\\}")
+                 (string-append "DESTINATION "
+                                #$output "/share/polkit-1/actions"))))))))
+    (home-page "https://github.com/FedeDP/Clightd")
+    (description "Bus interface for many display capabilities")
+    (synopsis "Clightd is a bus interface that lets you easily set/get screen
+ brightness, gamma temperature and display dpms state. Moreover, it enables
+ getting ambient brightness through webcam frames capture or ALS devices.
+
+ It works on X, Wayland and tty.")
+    (license license:gpl3)))
+
 (define-public herbstluftwm
   (package
     (name "herbstluftwm")
-- 
2.38.1

[bug#60013] [PATCH 2/3] gnu: Add clightd

[( via Guix-patches via]

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

Heya,

* gnu/packages/wm.scm (clightd): New variable.

--- a/gnu/packages/wm.scm
+++ b/gnu/packages/wm.scm

@@ -188,6 +193,70 @@ (define-public bspwm

+      #:tests? #f  ;;;tests not found

If there *are* actually tests, could you try to patch it to make them
work?

+            (lambda _
+              (substitute* "./CMakeLists.txt"
+                (("DESTINATION \\$\\{SYSTEM_BUS_DIR\\}")
+                 (string-append "DESTINATION "
+                                #$output "/share/dbus-1/system-services"))
+                (("DESTINATION \\$\\{POLKIT_ACTION_DIR\\}")
+                 (string-append "DESTINATION "
+                                #$output "/share/polkit-1/actions"))))

Maybe add these configure flags instead?

  (string-append "-DSYSTEM_BUS_DIR="
                 #$output "/share/dbus-1/system-services")
  (string-append "-DPOLKIT_ACTION_DIR="
                 #$output "/share/polkit-1/actions")

I'm not sure if that'll even work, I don't do CMake :)

+    (description "Bus interface for many display capabilities")
+    (synopsis "Clightd is a bus interface that lets you easily set/get screen
+ brightness, gamma temperature and display dpms state. Moreover, it enables
+ getting ambient brightness through webcam frames capture or ALS devices.

I think you got "description" and "synopsis" the wrong way round :)

    -- (

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[bug#60013] [PATCH 3/3] gnu: Add clight

[florhizome]

From: florhizome <florhizome@posteo.net>

* gnu/packages/wm.scm (clight): New variable.
---
 gnu/packages/wm.scm | 56 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm
index 919bcc9400..f7435e69f8 100644
--- a/gnu/packages/wm.scm
+++ b/gnu/packages/wm.scm
@@ -137,6 +137,7 @@ (define-module (gnu packages wm)
   #:use-module (gnu packages pcre)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages polkit)
+  #:use-module (gnu packages popt)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages pretty-print)
   #:use-module (gnu packages pulseaudio)
@@ -194,6 +195,61 @@ (define-public bspwm
     (license license:bsd-2)))
 
 
+(define-public clight
+  (package
+    (name "clight")
+    (version "4.9")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/FedeDP/clight")
+             (commit version)))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "09hf80kb01xwzr8p87d5r8rw1dq9jqdibyc7k2xc4z5v5j7hlnlv"))))
+    (build-system cmake-build-system)
+    (native-inputs (list pkg-config))
+    (inputs (list clightd
+                  dbus
+                  ddcutil
+                  elogind
+                  gsl
+                  geoclue
+                  libconfig
+                  libmodule
+                  popt
+                  upower))
+    (arguments
+     (list
+      #:tests? #f  ;no rule to make target test
+      #:configure-flags
+      #~(list (string-append "-DBASH_COMPLETIONS_DIR="
+                             #$output "/share/bash-completion/completions")
+              (string-append "-DFISH_COMPLETIONS_DIR="
+                             #$output "/share/fish/completions"))
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after
+              'patch-source-shebangs 'patch-target-dirs
+            (lambda _
+              (substitute*
+                  "./CMakeLists.txt"
+                (("DESTINATION \\$\\{SESSION_BUS_DIR\\}")
+                 (string-append "DESTINATION " #$output "/share/dbus-1/services"))
+                (("DESTINATION /etc/xdg/autostart")
+                 (string-append "DESTINATION " #$output "/etc/xdg/autostart"))
+                (("DESTINATION /usr/share/*")
+                 (string-append "DESTINATION " #$output "/share/*")))
+              #t)))))
+    (home-page "https://github.com/FedeDP/Clight")
+    (synopsis "User daemon utility to fully manage screens")
+    (description "Clight is a tiny C utility that can turn your webcam
+into a light sensor; moreover it supports a redshift-like gamma
+control, a screen dimmer and dpms settings.
+It is the userspace interface for clightd.")
+    (license license:gpl3)))
+
 (define-public clightd
   (package
     (name "clightd")
-- 
2.38.1

[bug#60013] [PATCH 3/3] gnu: Add clight

[( via Guix-patches via]

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

Heya,

* gnu/packages/wm.scm (clight): New variable.

--- a/gnu/packages/wm.scm
+++ b/gnu/packages/wm.scm

@@ -194,6 +195,61 @@ (define-public bspwm

+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after
+              'patch-source-shebangs 'patch-target-dirs
+            (lambda _
+              (substitute*
+                  "./CMakeLists.txt"
+                (("DESTINATION \\$\\{SESSION_BUS_DIR\\}")
+                 (string-append "DESTINATION " #$output "/share/dbus-1/services"))
+                (("DESTINATION /etc/xdg/autostart")
+                 (string-append "DESTINATION " #$output "/etc/xdg/autostart"))
+                (("DESTINATION /usr/share/*")
+                 (string-append "DESTINATION " #$output "/share/*")))
+              #t)))))

Same here; couldn't these be -D flags, except for the /usr/share/*
replacement?  Also, #T at the end isn't necessary anymore :)

+    (synopsis "User daemon utility to fully manage screens")

  (synopsis "Daemon for managing screens")

+    (description "Clight is a tiny C utility that can turn your webcam
+into a light sensor; moreover it supports a redshift-like gamma
+control, a screen dimmer and dpms settings.
+It is the userspace interface for clightd.")

  (description
   "@command{clight} is the interface to @command{clightd}.  It implements
  gamma control akin to @command{redshift}, a screen dimmer, and dpms settings.
  It can also turn your webcam into a light sensor.")

    -- (

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[( via Guix-patches via]

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

Heya,

On Mon Dec 12, 2022 at 8:31 PM GMT,  wrote:
> zimoun: yes I usually send an empty mail before a patchset wit multiple patches

There's a better way, using ``--cover-letter'':
<https://guix.gnu.org/manual/devel/en/html_node/Sending-a-Patch-Series.html#Multiple-Patches-1>

* gnu/packages/wm.scm (libmodule): New variable.

--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm

@@ -1300,6 +1300,28 @@ (define-public libdispatch

+       (uri (git-reference
+             (url "https://github.com/FedeDP/libmodule")
+             (commit version)))

Please add a ``file-name'':

  (file-name (git-file-name name version))

+    (arguments '(#:tests? #f)) ;tests not found

Use ``list'':

  (list #:tests? #f)

+    (description "Libmodule offers a small and simple C implementation of an
+ actor library that aims to let developers easily create modular C projects.")

There should be indentation between ``description'' and ``"..."''; also, some of
it seems a wee bit markety.  How about this?

  (description
   "Libmodule provides a C implementation of the actor model, aiming to help
  with the creation of modular C projects.")

    -- (

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[zimoun]

Hi,

On Mon, 12 Dec 2022 at 20:31, florhizome@posteo.net wrote:

> zimoun: yes I usually send an empty mail before a patchset wit
> multiple patches

Two minor opinionated comments. :-) Maybe instead of an empty mail
before a patch set, you could send a cover letter.  For instance, the
most simple as template is provided by:

    git format-patch --cover-letter -3 --base=origin/master

which creates 0000-cover-letter.patch; and it looks like:

--8<---------------cut here---------------start------------->8---
From e84042fdd4ac2f903224e1b5f366537a22b4d593 Mon Sep 17 00:00:00 2001
From: zimoun <zimon.toutoune@gmail.com>
Date: Tue, 13 Dec 2022 00:42:04 +0100
Subject: [PATCH 0/3] *** SUBJECT HERE ***

*** BLURB HERE ***

Ricardo Wurmus (3):
  gnu: python-flasgger: Update to 0.9.5.
  gnu: seqmagick: Update to 0.8.4.
  gnu: cnvkit: Update to 0.9.9.

 gnu/packages/bioinformatics.scm | 31 +++++++++++++++++++---------
 gnu/packages/python-xyz.scm     | 36 +++++++++++++++++++--------------
 2 files changed, 42 insertions(+), 25 deletions(-)


base-commit: 8c197f990b7ab60e4524567d628d78ea62973c11
-- 
2.35.1
--8<---------------cut here---------------end--------------->8---



Last, just to point that,

> base-commit: 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
> prerequisite-patch-id: 36ae907c0ae2cbc001f774c0514ab217855270c2

[...]

> prerequisite-patch-id: 709b4522fd225f3c28bb2721e5dd9a20014ce131

these prerequisite could lead to trouble when applying this patch set.


Anyway, thanks for the patch set. :-)

Cheers,
simon

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[Florian]

Hi all,
thanks for your comments. 
A revamped patch-set should be on the way soon, depending mostly on
your's truly input on how to go on with the tests, but see below :)

T B-R: That's how it was! Although these days my mails were as fast as a
couple hours to appear on the mailing list, who could complain ;)

unmatched-paren: I checked the projects for configure flags before I
sent them. Unfortunately I am pretty sure, the remaining patches are not
avoidable, since the variables are set by cmakes pkg-config
queries, but correct me if wrong! I am no expert in any build system,
but these things are often detectable I think :). (btw if there was an effort to write a blog post to document
problems for nix/guix packaging, the use of pkg-config queries those should be explicitly
mentioned, it's a pest!).

About tests:
I also can't find any tests in clight and clightd's repos. At least for
libmodule I found how to build and enable tests
https://github.com/FedeDP/libmodule/tree/16435d57b7600610313dc21301f3b5717480a3a8/tests
So by setting this flag and packing valgrind and cmocka into
'native-inputs', two tests will be built and run. The one using valgrind
will fail though:

starting phase `check'
Running tests...
/gnu/store/j65q3aw414010gdfvmsynwpzfb2jyyd3-cmake-minimal-3.21.4/bin/ctest --force-new-ctest-process 
Test project /tmp/guix-build-libmodule-5.0.1.drv-0/build
    Start 1: ModuleTest
1/2 Test #1: ModuleTest .......................   Passed    1.21 sec
    Start 2: ModuleTest_valgrind
2/2 Test #2: ModuleTest_valgrind ..............***Failed    0.02 sec
==258== Memcheck, a memory error detector
==258== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==258== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==258== Command: ./ModuleTest
==258== 

valgrind:  Fatal error at startup: a function redirection
valgrind:  which is mandatory for this platform-tool combination
valgrind:  cannot be set up.  Details of the redirection are:
valgrind:  
valgrind:  A must-be-redirected function
valgrind:  whose name matches the pattern:      strlen
valgrind:  in an object with soname matching:   ld-linux-x86-64.so.2
valgrind:  was not found whilst processing
valgrind:  symbols from the object with soname: ld-linux-x86-64.so.2
valgrind:  
valgrind:  Possible fixes: (1, short term): install glibc's debuginfo
valgrind:  package on this machine.  (2, longer term): ask the packagers
valgrind:  for your Linux distribution to please in future ship a non-
valgrind:  stripped ld.so (or whatever the dynamic linker .so is called)
valgrind:  that exports the above-named function using the standard
valgrind:  calling conventions for this platform.  The package you need
valgrind:  to install for fix (1) is called
valgrind:  
valgrind:    On Debian, Ubuntu:                 libc6-dbg
valgrind:    On SuSE, openSuSE, Fedora, RHEL:   glibc-debuginfo
valgrind:  
valgrind:  Note that if you are debugging a 32 bit process on a
valgrind:  64 bit system, you will need a corresponding 32 bit debuginfo
valgrind:  package (e.g. libc6-dbg:i386).
valgrind:  
valgrind:  Cannot continue -- exiting now.  Sorry.



50% tests passed, 1 tests failed out of 2

Total Test time (real) =   1.23 sec

And I'm not sure how to help with that. including (list glibc "debug")
didn't do it. Do we need really need this test?
The next question is then if the tests that are automatically run are
those mentioned in the README on the projects wiki (see url
above). Chime-in!

zimoun: I think I have been leaving that indentation after (description
in everything I contributed so far. I seriously hope I don't need to rework all of
those just for that. But for the future, I know now ;)
These also would be those "prerequisite-patch-ids" hanging around, that
are generated by the --base=auto flag I use with git send-email like a
good manual follower. I don't want to operate on multiple guix repos or branches, I find
the workflow to keep one updated time-consuming enough so far... So I
guess, to be frank, what can I actually do about that?

Side Note: Of course this list would become smaller the more previous patches are
actually merged *cough cough* most of whom are simple, one-hunk patches
that should require the least amount of time :)

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[zimoun]

Hi Florian,

On Tue, 13 Dec 2022 at 11:51, Florian <florhizome@posteo.net> wrote:

> These also would be those "prerequisite-patch-ids" hanging around, that
> are generated by the --base=auto flag I use with git send-email like a
> good manual follower. I don't want to operate on multiple guix repos or branches, I find
> the workflow to keep one updated time-consuming enough so far... So I
> guess, to be frank, what can I actually do about that?

I understand.  My point is just that it can be hard to be sure that the
patch set correctly applies; because some prerequisite-patch-ids could
modifies something without being included.  For instance, I think it is
what is happening with this current patch set.  On the top of,

    base-commit: 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0

from the master branch, PATCH 1/3 correctly applies but I am failing to
apply PATCH 2/3.

--8<---------------cut here---------------start------------->8---
error: patch failed: gnu/packages/wm.scm:102
error: gnu/packages/wm.scm: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: gnu: Add clightd
Patch failed at 0001 gnu: Add clightd
--8<---------------cut here---------------end--------------->8---

Well, I understand your workflow, I guess.  My point is just to mention
that creating a branch with Git costs nothing.  For instance,

--8<---------------cut here---------------start------------->8---
git checkout -d example
git reset --soft 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
git cherry-pick 5ad571d5a145a7a33ec9241bf75f25cf4864e629
git format-patch -1 --cover-letter --base=0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
git checkout master
git branch -D example
--8<---------------cut here---------------end--------------->8---

where 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0 is ’origin/master’ when
you created this patch set.  And where
5ad571d5a145a7a33ec9241bf75f25cf4864e629 is the commit adding libmodule.

From my point of view, it changes almost nothing for you and, IMHO, it
helps the review, both the human and the automated [1] ones.

Obviously, it is an opinionated minor comment. :-) And for sure, the
most important is that you send your contributions–meaning, the way that
is comfortable for you.

1: https://qa.guix.gnu.org/issue/60013

Cheers,
simon

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[Florian]

On 2022-12-15, 12:31 +0100, zimoun <zimon.toutoune@gmail.com> wrote:

> Well, I understand your workflow, I guess.  My point is just to mention
> that creating a branch with Git costs nothing.

Creating a branch, yes, but continuously rebasing them all and recompiling before
working on each single commission: yes (building guix is pretty time
intensive). This is where I think guix lacks a proper CI/CD. That contributors
even have to check if a patch applies, if it builds and run guix lint
seems paradox to me, especially with the capacity problems that already exist
when accepting submissions.

I think the problem you have comes from multiple patches of mine being in
wm.scm which has also made rebasing more complicated. Git really seems
to struggle when patches have hunks in one file that might touch each other,
that I haven't really understood how to avoid. but I can check
one more time. Maybe we could have a look at how the patch is failing?

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[( via Guix-patches via]

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

On Tue Dec 20, 2022 at 4:33 PM GMT, Florian wrote:
> On 2022-12-15, 12:31 +0100, zimoun <zimon.toutoune@gmail.com> wrote:
> Creating a branch, yes, but continuously rebasing them all and recompiling before
> working on each single commission: yes (building guix is pretty time
> intensive).

Maybe try using ``guix worktree'' to create new directories for each branch checkout.

> This is where I think guix lacks a proper CI/CD. That contributors
> even have to check if a patch applies, if it builds and run guix lint
> seems paradox to me, especially with the capacity problems that already exist
> when accepting submissions.

Have you seen <https://qa.guix.gnu.org>?  :)

    -- (

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[bug#60013] [PATCH 1/3] gnu: Add libmodule

[zimoun]

Hi,

On Tue, 20 Dec 2022 at 16:33, Florian <florhizome@posteo.net> wrote:

> Creating a branch, yes, but continuously rebasing them all and recompiling before
> working on each single commission: yes (building guix is pretty time
> intensive). This is where I think guix lacks a proper CI/CD. That contributors
> even have to check if a patch applies, if it builds and run guix lint
> seems paradox to me, especially with the capacity problems that already exist
> when accepting submissions.

Are you aware about <https://qa.guix.gnu.org/patches>?

And for what it is worth, your submission [1] fails with the Guix CI/CD.
Probably because your patches do not apply correctly.

1: <https://qa.guix.gnu.org/issue/60013>

What I was suggesting is not to rebase and not to rebuild but instead to
cherry-pick and then correctly format patches.  Please note that this
comment applies equally for any workflow – even for some PR workflow à
la Git{Hub,Lab}.


> I think the problem you have comes from multiple patches of mine being in
> wm.scm which has also made rebasing more complicated. Git really seems
> to struggle when patches have hunks in one file that might touch each other,
> that I haven't really understood how to avoid. but I can check
> one more time. Maybe we could have a look at how the patch is failing?

As pointed in [1], the patch just does not apply,

--8<---------------cut here---------------start------------->8---
error: patch failed: gnu/packages/wm.scm:102
error: gnu/packages/wm.scm: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: gnu: Add clightd
Patch failed at 0001 gnu: Add clightd
--8<---------------cut here---------------end--------------->8---

because of gnu/packages/wm.scm:102.  From my understanding, it is not
about rebasing, it is about which state of the file the diff must apply
against.

What I was suggesting is to extract these 3 patches using cherry-pick
and so format the patches against a clean origin/master.  For instance,

--8<---------------cut here---------------start------------->8---
git checkout -d example
git reset --soft 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
git cherry-pick 5ad571d5a145a7a33ec9241bf75f25cf4864e629
git cherry-pick <commit-hash-adding-clightd>
git cherry-pick <commit-hash-adding-clight>
git format-patch -3 --cover-letter --base=0ce1f82e5aaac951b21d579eb46bf75cfe6713c0
git checkout master
git branch -D example
--8<---------------cut here---------------end--------------->8---

where 0ce1f82e5aaac951b21d579eb46bf75cfe6713c0 is a clean
’origin/master’ when you created this patch set.  And where
5ad571d5a145a7a33ec9241bf75f25cf4864e629 is the commit adding libmodule.

It changes nothing for you, IMHO, and it helps for having patches which
correctly apply; although you still have other unreleated patches.


1: <https://yhetil.org/guix/86pmckdhts.fsf@gmail.com>

Cheers,
simon