[PATCH] gnu: Add clight

Hi,

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

It seems empty. :-)

Cheers,

simon

---

gnu/packages/wm.scm | 69 +++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 69 insertions(+)

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

---

gnu/packages/c.scm | 22 ++++++++++++++++++++++

1 file changed, 22 insertions(+)

---

gnu/packages/wm.scm | 56 +++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 56 insertions(+)

zimoun ???

They were probably waiting for a bug number and it (or their

patches) haven't arrived yet.

Ah, debbugs,

T G-R

Heya,

On Mon Dec 12, 2022 at 8:31 PM GMT, wrote:

There's a better way, using ``--cover-letter'':

--- 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.")

-- (

Heya,

--- 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 :)

-- (

Heya,

--- 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.")

-- (

Hi,

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

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:

Last, just to point that,

[...]

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

Anyway, thanks for the patch set. :-)

Cheers,

simon

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

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 :)

Hi Florian,

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

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.

Well, I understand your workflow, I guess. My point is just to mention

that creating a branch with Git costs nothing. For instance,

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

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). 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?

On Tue Dec 20, 2022 at 4:33 PM GMT, Florian wrote:

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

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

-- (

Hi,

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

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}.

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

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,

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