Re: G-expressions and chroot environment? (was Re: branch master updated: gnu: Add passff.)
Hi, On Fri, 03 Nov 2023 at 19:46, Simon Tournier wrote: > However, there is still something that I am missing. The derivations > tracks all and that’s expected; thanks G-expression machinery. :-) > However, I miss how the builder works with the chrooted environment if > nothing is passed to it. Thanks to chat with civodul, now all is clear. :-) The isolated environment is populated using the list provided by the derivation. Somehow, the G-expression machinery tracks all for helping in constructing the derivation, then the inputs of that derivation are put inside the isolated environment where the builder Guile script runs. Cool! Cheers, simon
G-expressions and chroot environment? (was Re: branch master updated: gnu: Add passff.)
Hi,
On Sat, 28 Oct 2023 at 17:05, Clément Lassieur wrote:
>> ./pre-inst-env guix show passff-host
>> name: passff-host
>> version: 1.2.3
>> outputs:
>> + out: everything
>> systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux
>> riscv64-linux
>> + i686-linux armhf-linux i586-gnu powerpc-linux
>> dependencies:
>
> I imagine it's a bug in `guix show`?
It is not a bug of ’guix show’ because ’guix show’ accesses to the
fields of the package record. And Clément’s patch is an “abuse” of the
G-expressions machinery. :-)
> As doc says:
>
>• Gexps carry information about the packages or derivations they
> refer to, and these dependencies are automatically added as inputs
> to the build processes that use them.
Well, this is correct from my understanding. ;-)
However, there is still something that I am missing. The derivations
tracks all and that’s expected; thanks G-expression machinery. :-)
However, I miss how the builder works with the chrooted environment if
nothing is passed to it.
The derivation reads,
--8<---cut here---start->8---
Derive
([("out","/gnu/store/0amanwyzx3jylyw7bz5nmszpybxll8ww-passff-host-1.2.3","","")]
,[("/gnu/store/070vbkzbs0dn6w9mhz0xw8fi5hfp92rg-make-4.3.drv",["out"])
,("/gnu/store/2i4781y3mmnm2jlx3awa4mwbqam2ar80-python-3.10.7.drv",["out"])
,("/gnu/store/ax7wdlbxhcz7w8nfyrxkb1pqai80niw6-sed-4.8.drv",["out"])
,("/gnu/store/gb247cil5nlnx175dhqmgg67q7ng7n2h-which-2.21.drv",["out"])
,("/gnu/store/ghwl0z5ci5sssbrzixxji8l0x3j9i3dv-bash-minimal-5.1.16.drv",["out"])
,("/gnu/store/mqmnsly3nm0a7hj46apf2hfm7j8wk56h-module-import-compiled.drv",["out"])
,("/gnu/store/raay3plnbzadwqc0yv8yw8pjr929pkqd-coreutils-9.1.drv",["out"])
,("/gnu/store/rnphhzpwkz82zf1il1cg52041myvp3d4-password-store-1.7.4.drv",["out"])
,("/gnu/store/y6871hl8lklcslvw57wj4bnyysxlv2np-passff-host-1.2.3-checkout.drv",["out"])
,("/gnu/store/y9l0jnyxssx1glbyg3cav78js2fm7j50-grep-3.8.drv",["out"])
,("/gnu/store/zraigp7miin3vzr5dcbr4i9rvds0i07r-guile-3.0.9.drv",["out"])]
,["/gnu/store/8nam67byqnpvbfn4anpgg5pb2qrqhs3v-passff-host-1.2.3-builder","/gnu/store/pj751v3199vmv6i6sf0szp185ryzcfdg-module-import"]
,"x86_64-linux","/gnu/store/g8p09w6r78hhkl2rv1747pcp9zbk6fxv-guile-3.0.9/bin/guile",["--no-auto-compile","-L","/gnu/store/pj751v3199vmv6i6sf0szp185ryzcfdg-module-import","-C","/gnu/store/2gbsk55kwag577skxwsxrfy3l4cl03xh-module-import-compiled","/gnu/store/8nam67byqnpvbfn4anpgg5pb2qrqhs3v-passff-host-1.2.3-builder"]
,[("out","/gnu/store/0amanwyzx3jylyw7bz5nmszpybxll8ww-passff-host-1.2.3")])
--8<---cut here---end--->8---
However the builder reads,
--8<---cut here---start->8---
(begin
(define %build-inputs
(quote
(("source" .
"/gnu/store/fjnkcv14qb61623lm16kq1mgb4bsxivl-passff-host-1.2.3-checkout"
(define %outputs
(list
(cons "out"
((@
(guile)
getenv)
"out"
(define %output
(assoc-ref %outputs "out"))
(begin
(use-modules
(guix build utils))
(setenv "PATH"
(string-join
(list
"/gnu/store/yr39rh6wihd1wv6gzf7w4w687dwzf3vb-coreutils-9.1/bin"
"/gnu/store/ixr7c3jadiqg640b8pz3njqhhm5zzmvj-grep-3.8/bin"
"/gnu/store/sj794a2709pxsi4mgvi619qdpi1g32aa-password-store-1.7.4/bin"
"/gnu/store/dy3xh053ahkhrp2jamggq8cpsyvp8mg0-python-3.10.7/bin"
"/gnu/store/fyy3wkjkix16sb1ginqw2kbji74cwl2b-sed-4.8/bin"
"/gnu/store/6vxk0i5j9w8mik4l6gx3cbw33f9x4l24-which-2.21/bin")
":"))
(copy-recursively
"/gnu/store/fjnkcv14qb61623lm16kq1mgb4bsxivl-passff-host-1.2.3-checkout" ".")
(substitute* "src/install_host_app.sh"
(("#!/usr/bin/env sh")
"/gnu/store/9vw5slrffp27rzy2i2plnw7xfqjyk7m4-bash-minimal-5.1.16/bin/sh")
(("(TARGET_DIR_FIREFOX=).*" all var)
(string-append var
((@
(guile)
getenv)
"out")
"/lib/icecat/native-messaging-hosts")))
(invoke "/gnu/store/vq4g8390wbz2434m678v010mkpnkjb2w-make-4.3/bin/make"
(string-append "VERSION=" "1.2.3")
"install-unix")))
--8<---cut here---end--->8---
and I would have expect that it fails because of the isolated
environment.
How is it possible that the builder script is able to run that?
For another example using the gnu-build system.
--8<---cut here---start->8---
(define-module (appendix)
#:use-module (guix packages)
#:use-module (gnu packages base)
#:use-module (guix utils)
#:use-module (guix gexp)
#:use-module (gnu packages emacs))
(define-public bye
(package
(inherit hello)
(name "bye")
(arguments
(list
#:phases
#~(modify-phases %standard-phases
Re: branch master updated: gnu: Add passff.
See here for the patches. https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66868 Thanks, Clément
Re: branch master updated: gnu: Add passff.
On Mon, Oct 30 2023, Clément Lassieur wrote: > Hi Kaelyn, > > On Sat, Oct 28 2023, Kaelyn wrote: > >> I agree about this. When I packaged passff-host locally some time ago, I saw > I'm happy I'm not alone packaging Icecat extensions :) > >> it has a runtime dependency on python and also needs to be able to find the >> pass binary. > > My patch also has dependencies on python and password-store. You can > have a look at documentation about G-Expressions to understand how. It's indeed better if the inputs are explicit (see conversation with John) so I amended my patch again to change a few things. Now it has fewer dependencies (password-store and python, as you rightfully said above), and they all are explicit. >> I've attached the bare (unfinished/unpolished) package definition >> extracted from my local channel and attached it, for if it is of assistance >> to >> folks. > > It's a bit late for passff-host because it's already there (and it works > well), but if you need help for packaging other Icecat extensions, I'd > be glad to help. > >> My definition tries to embed a sane path for finding pass with a >> default of the path to the password-store package it was built against, and >> also tries to copy the passff.json into the correct browser folder for > > [...] > >> ;; FIXME: The passff.json in etc/ needs to go into a browser-dependent >> ;; location to work with that specific browser. How to install it to the >> ;; right location needs to be figured out and documented. > > The Icecat browser-dependent location did not exist before > https://git.savannah.gnu.org/cgit/guix.git/commit/?id=25043e01b6cb6696ffdc6cdedd9cdf8235bb695c. > > Now it is in /lib/icecat, e.g. /lib/icecat/native-messaging-hosts > > [...] > >> ;; NOTE: python-build-system is used instead of copy-build-system to >> ;; automatically pick up the Python 3 dependency and to wrap the >> installed >> ;; Python script. > > `trivial-build-system` is enough, most of the work is done by the > Makefile and install_host_app.sh. Well this was wrong :) See my updated patch where I use copy-build-system and don't use Makefile and install_host_app.sh anymore... Thanks, Clément
Re: branch master updated: gnu: Add passff.
Hi John,
Thanks for taking the time to write this message.
On Tue, Oct 31 2023, John Kehayias wrote:
> Right, it uses gexps but I think here the better and more explicit
> style would be to use inputs/native-inputs. Then instead of
> referencing directly like #$ use
> #$(this-package-input "package-name") to get the store path. This I
> think is clearer and I believe better allows for inheritance,
> input-rewriting, and so on.
You are right, I'll amend my patch, taking this into account.
I believe `guix lint` should say something about it though, maybe warn
when using an input that is not explicitly added in the `input` field.
Those errors are hard to catch and are in our code base at several
places already.
I've also noticed there is some subtle in-between style in our code
base: people adding x as input but still using #$x in the package
definition instead of using #$(this-package-input "x"). It's wrong
because in this case input rewriting does not work.
G-Expressions allow for weird things. Here's another weird pattern from
our code base: going from (inputs `("foo", ,(origin ...))) to #$(origin
...) without inputs. This removes input rewriting too.
> Feel free for anyone else to chime in on this point, I'm always
> looking to learn to improve my own packaging and review, but this is
> what I understand is preferred when possible.
>
>>> Was this change sent as a patch to guix-patches?
>>
>> No it wasn't.
>
> The mantra I've heard, and agree with, is that the
> trivial-build-system is anything but trivial. Not saying it wasn't the
> best choice here, or has anything to do with the above points, but
> thought it worth mentioning for anyone else.
Agreed.
> But this is also why I think it would have been better to have it go
> through review. I see there's been several followup commits to improve
> the style and fix things which also could have been avoided. Not a
> huge deal perhaps, but I would err on the side of review for something
> like this.
I thought my patch was trivial, but I was obviously wrong since I
(indeed) amended it twice, and now a third time.
> Of course, thanks for the contribution!
Thanks for the kind words :)
Clément
Re: branch master updated: gnu: Add passff.
Hi Clément, On Sat, Oct 28, 2023 at 05:05 PM, Clément Lassieur wrote: > On Sat, Oct 28 2023, Christopher Baines wrote: > >> This passff-host package looks a bit odd to me, one thing to mention is >> that guix show says it has no dependencies, but I don't think that's >> correct: >> >> ./pre-inst-env guix show passff-host >> name: passff-host >> version: 1.2.3 >> outputs: >> + out: everything >> systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux >> riscv64-linux >> + i686-linux armhf-linux i586-gnu powerpc-linux >> dependencies: > > I imagine it's a bug in `guix show`? As doc says: > >• Gexps carry information about the packages or derivations they > refer to, and these dependencies are automatically added as inputs > to the build processes that use them. > Right, it uses gexps but I think here the better and more explicit style would be to use inputs/native-inputs. Then instead of referencing directly like #$ use #$(this-package-input "package-name") to get the store path. This I think is clearer and I believe better allows for inheritance, input-rewriting, and so on. Feel free for anyone else to chime in on this point, I'm always looking to learn to improve my own packaging and review, but this is what I understand is preferred when possible. >> Was this change sent as a patch to guix-patches? > > No it wasn't. The mantra I've heard, and agree with, is that the trivial-build-system is anything but trivial. Not saying it wasn't the best choice here, or has anything to do with the above points, but thought it worth mentioning for anyone else. But this is also why I think it would have been better to have it go through review. I see there's been several followup commits to improve the style and fix things which also could have been avoided. Not a huge deal perhaps, but I would err on the side of review for something like this. Of course, thanks for the contribution! John
Re: branch master updated: gnu: Add passff.
Hi Kaelyn, On Sat, Oct 28 2023, Kaelyn wrote: > I agree about this. When I packaged passff-host locally some time ago, I saw I'm happy I'm not alone packaging Icecat extensions :) > it has a runtime dependency on python and also needs to be able to find the > pass binary. My patch also has dependencies on python and password-store. You can have a look at documentation about G-Expressions to understand how. > I've attached the bare (unfinished/unpolished) package definition > extracted from my local channel and attached it, for if it is of assistance to > folks. It's a bit late for passff-host because it's already there (and it works well), but if you need help for packaging other Icecat extensions, I'd be glad to help. > My definition tries to embed a sane path for finding pass with a > default of the path to the password-store package it was built against, and > also tries to copy the passff.json into the correct browser folder for [...] > ;; FIXME: The passff.json in etc/ needs to go into a browser-dependent > ;; location to work with that specific browser. How to install it to the > ;; right location needs to be figured out and documented. The Icecat browser-dependent location did not exist before https://git.savannah.gnu.org/cgit/guix.git/commit/?id=25043e01b6cb6696ffdc6cdedd9cdf8235bb695c. Now it is in /lib/icecat, e.g. /lib/icecat/native-messaging-hosts [...] > ;; NOTE: python-build-system is used instead of copy-build-system to > ;; automatically pick up the Python 3 dependency and to wrap the installed > ;; Python script. `trivial-build-system` is enough, most of the work is done by the Makefile and install_host_app.sh. Thanks, Clément
Re: branch master updated: gnu: Add passff.
--- Original Message ---
On Saturday, October 28th, 2023 at 8:05 AM, Clément Lassieur
wrote:
>
>
> On Sat, Oct 28 2023, Christopher Baines wrote:
>
> > This passff-host package looks a bit odd to me, one thing to mention is
> > that guix show says it has no dependencies, but I don't think that's
> > correct:
I agree about this. When I packaged passff-host locally some time ago, I saw it
has a runtime dependency on python and also needs to be able to find the pass
binary. I've attached the bare (unfinished/unpolished) package definition
extracted from my local channel and attached it, for if it is of assistance to
folks. My definition tries to embed a sane path for finding pass with a default
of the path to the password-store package it was built against, and also tries
to copy the passff.json into the correct browser folder for Chromium, Firefox,
and Icecat based on the packaged install_host_app.sh script (note: I have only
used it with Icecat).
Cheers,
Kaelyn
> >
> > ./pre-inst-env guix show passff-host
> > name: passff-host
> > version: 1.2.3
> > outputs:
> > + out: everything
> > systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux
> > riscv64-linux
> > + i686-linux armhf-linux i586-gnu powerpc-linux
> > dependencies:
>
>
> I imagine it's a bug in `guix show`? As doc says:
>
> • Gexps carry information about the packages or derivations they
> refer to, and these dependencies are automatically added as inputs
> to the build processes that use them.
>
> > Was this change sent as a patch to guix-patches?
>
>
> No it wasn't.
I(define-public passff-host
(package
(name "passff-host")
(version "1.2.3")
(home-page "https://github.com/passff/passff-host/;)
(source
(origin
(method git-fetch)
(uri (git-reference (url home-page)
(commit version)))
(file-name (git-file-name name version))
(sha256
(base32
"1p18l1jh20x4v8dj64z9qjlp96fxsl5h069iynxfpbkzj6hd74yl"))
))
;; NOTE: python-build-system is used instead of copy-build-system to
;; automatically pick up the Python 3 dependency and to wrap the installed
;; Python script.
;; FIXME: The passff.json in etc/ needs to go into a browser-dependent
;; location to work with that specific browser. How to install it to the
;; right location needs to be figured out and documented.
(build-system python-build-system)
(arguments
(list #:tests? #f ; There are no tests
#:phases
#~(modify-phases %standard-phases
(replace 'build
(lambda _
;; TODO? Add password-store as an input and embed the store
;; path to the "pass" executable.
(substitute* "src/passff.py"
(("_VERSIONHOLDER_") #$(package-version this-package))
;; (("\"pass\"") (string-append
;;"\""
;;#$(this-package-input "password-store")
;;"/bin/pass\""))
(("\"PATH\": .*")
(string-join (list
"\"PATH\""
" \"/run/current-system/profile/bin"
"/run/booted-system/profile/bin"
(string-append
#$(this-package-input "password-store")
"/bin\",\n"))
":"))
)))
(replace 'install
(lambda _
(let ((etc (string-append #$output "/etc"))
(libexec (string-append #$output "/libexec")))
;; Install the host script in libexec
(install-file "src/passff.py" libexec)
;; Insert the script path and install the (example)
;; native host manifest to etc
(substitute* "src/passff.json"
(("PLACEHOLDER") (string-append libexec "/passff.py")))
(for-each
(lambda (dir)
(install-file "src/passff.json" dir))
(list etc ;; Generic location for easier access
;; Chromium location based on src/install_host_app.sh
(string-append etc "/chromium/native-messaging-hosts")
;; Firefox location based on src/install_host_app.sh
(string-append #$output "/lib/mozilla/native-messaging-hosts")
;; Icecat location derived from the above Firefox location
(string-append #$output "/lib/icecat/native-messaging-hosts")))
)))
)))
(inputs
Re: branch master updated: gnu: Add passff.
On Sat, Oct 28 2023, Christopher Baines wrote: > This passff-host package looks a bit odd to me, one thing to mention is > that guix show says it has no dependencies, but I don't think that's > correct: > > ./pre-inst-env guix show passff-host > name: passff-host > version: 1.2.3 > outputs: > + out: everything > systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux > riscv64-linux > + i686-linux armhf-linux i586-gnu powerpc-linux > dependencies: I imagine it's a bug in `guix show`? As doc says: • Gexps carry information about the packages or derivations they refer to, and these dependencies are automatically added as inputs to the build processes that use them. > Was this change sent as a patch to guix-patches? No it wasn't.
Re: branch master updated: gnu: Add passff.
guix-comm...@gnu.org writes: > This is an automated email from the git hooks/post-receive script. > > snape pushed a commit to branch master > in repository guix. > > The following commit(s) were added to refs/heads/master by this push: > new 6c894b7a1a gnu: Add passff. > 6c894b7a1a is described below > > commit 6c894b7a1a6a7a0f7c51a44136bba00dc0b5250c > Author: Clément Lassieur > AuthorDate: Tue Oct 17 16:53:57 2023 +0200 > > gnu: Add passff. > > * gnu/packages/browser-extensions.scm (passff-host): New variable. > (passff): New variable. > > Change-Id: I0f6f4b0c319e5cffd0940421a4d8bdc73d8d806b > --- > gnu/packages/browser-extensions.scm | 76 > + > 1 file changed, 76 insertions(+) This passff-host package looks a bit odd to me, one thing to mention is that guix show says it has no dependencies, but I don't think that's correct: ./pre-inst-env guix show passff-host name: passff-host version: 1.2.3 outputs: + out: everything systems: x86_64-linux mips64el-linux aarch64-linux powerpc64le-linux riscv64-linux + i686-linux armhf-linux i586-gnu powerpc-linux dependencies: Was this change sent as a patch to guix-patches? Thanks, Chris signature.asc Description: PGP signature
