bug#44254: Performance of package input rewriting
Hi, > Yes, that’s a possible culprit. Try passing #:deep? #f if it works for > your use case. Yeah, that brings it down to ~8s, which is still alot. > Another thing to look at is the object graph (as show by ‘guix > graph’). Input rewriting can duplicate parts of the graph, which in > turn defeats package->derivation memoization. Just looking at the > number of nodes in the graph can give hints. Aha, it’s 913 nodes without rewriting, 13916 with rewriting (#:deep? #t) and 4286 with rewriting (#:deep? #f) as determined by a rather ad-hoc `guix graph -L . -t package python-jupyterlab | grep 'shape = box' | wc -l`. That seems way too much. Does that mean I’m using package rewriting in the wrong way or is that a bug? Unfortunately I don’t have a short reproducer right now. I’ll look at the graph more closely to figure out which parts are actually duplicated. Maybe I can create a reproducing testcase with more information. Cheers, Lars signature.asc Description: PGP signature
bug#44327: `guix install` doesn't warn about collison in profile
In commit ba60bbd4370570ff03a16c63af051be06f22658e. Try command guix install emacs && guix install emacs-xwidgets These two packages are conflict with each other, but I can't see any warning message emitted(It should emit some because profile-derivation use union-build). I also suggest to raise an error when conflict detected during building profile to force user to resolve it. -- Retrieve my PGP public key: https://meta.sr.ht/~citreu.pgp Zihao signature.asc Description: PGP signature
bug#44327: `guix install` doesn't warn about collison in profile
Dear, Thank you for the report. On Fri, 30 Oct 2020 at 19:17, Zhu Zihao wrote: > In commit ba60bbd4370570ff03a16c63af051be06f22658e. Try command > > guix install emacs && guix install emacs-xwidgets I do not know if this is really a collision since it is sequential. Well, I agree that a warning message should be displayed. However, note that, $ guix time-machine --commit=ba60bbd4 \ -- install emacs emacs-xwidgets -p /tmp/foo $ guix time-machine --commit=ba60bbd4 \ -- install emacs-xwidgets emacs -p /tmp/bar readlink -f /tmp/{foo,bar}/bin/emacs /gnu/store/xy38cgnr21j2jsb5mnn9cf91d6q67as2-emacs-xwidgets-27.1/bin/emacs-27.1 /gnu/store/1zwmmfy1d213mrqixl36ckbzghkpqdmf-emacs-27.1/bin/emacs which is a bug. AFAIU. Same with “guix package -i”. > I also suggest to raise an error when conflict detected during building > profile to force user to resolve it. It should be already the case. :-) All the best, simon
bug#44327: `guix install` doesn't warn about collison in profile
Hi, Zhu Zihao skribis: > In commit ba60bbd4370570ff03a16c63af051be06f22658e. Try command > > guix install emacs && guix install emacs-xwidgets > > These two packages are conflict with each other, but I can't see any > warning message emitted(It should emit some because profile-derivation > use union-build). ‘union-build’ emits a warning, but it’s only visible in build logs. > I also suggest to raise an error when conflict detected during building > profile to force user to resolve it. Currently, ‘guix install’ errors out if you try to install two same-named packages with a different version number or a different store file name (typically via propagated inputs). Here, ‘emacs’ and ‘emacs-xwidgets’ have different names, so it doesn’t complain. Perhaps we should offer a way to annotate packages as conflicting with one another? Thanks, Ludo’.
bug#44327: `guix install` doesn't warn about collison in profile
On Fri, 30 Oct 2020 at 17:20, Ludovic Courtès wrote: > Currently, ‘guix install’ errors out if you try to install two > same-named packages with a different version number or a different store > file name (typically via propagated inputs). > > Here, ‘emacs’ and ‘emacs-xwidgets’ have different names, so it doesn’t > complain. > > Perhaps we should offer a way to annotate packages as conflicting with > one another? Why? The solution seems to check the path names. Here, the 2 packages provide 'bin/emacs'; which is the conflict. However, this could be implemented with care otherwise it will slow down. I have not check 'union-build' but maybe it is already the case. :-) All the best, simon
bug#44196: [PATCH v5 3/4] system: Do not depend on locale folder generated by
Changes w.r.t. previous version: - Call normalize-file (the parameters were there but I lost the call). - Only call search when there is no image configured. >From 3eb494947ed863e60d3d49d4ac4a982b1f9237e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?= Date: Sat, 24 Oct 2020 20:36:21 +0200 Subject: [PATCH] system: Do not depend on locale folder generated by grub-install. * gnu/bootloader/grub.scm (define-module): Use (guix packages). (grub-locale-folder): New computed folder. (grub-configuration-file)[locale-config]: Use grub-locale-folder only when a locale is needed. --- gnu/bootloader/grub.scm | 56 + 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm index 5508319d3b..2604f59117 100644 --- a/gnu/bootloader/grub.scm +++ b/gnu/bootloader/grub.scm @@ -25,6 +25,7 @@ (define-module (gnu bootloader grub) #:use-module (guix build union) + #:use-module (guix packages) #:use-module (guix records) #:use-module (guix store) #:use-module (guix utils) @@ -142,6 +143,24 @@ file with the resolution provided in CONFIG." (image->png image #:width width #:height height)) (_ #f) +(define (grub-locale-folder grub) + "Generate a folder with the locales from GRUB." + (define builder +#~(begin +(use-modules (ice-9 ftw)) +(let ((locale (string-append #$grub "/share/locale")) + (out#$output)) + (mkdir out) + (chdir out) + (for-each (lambda (lang) + (let ((file (string-append locale "/" lang + "/LC_MESSAGES/grub.mo")) +(dest (string-append lang ".mo"))) +(when (file-exists? file) + (copy-file file dest +(scandir locale) + (computed-file "grub-boot-locale" builder)) + (define* (eye-candy config store-device store-mount-point #:key store-directory-prefix port) "Return a gexp that writes to PORT (a port-valued gexp) the 'grub.cfg' part @@ -404,18 +423,33 @@ menuentry ~s { #:port #~port))) (define locale-config -#~(let ((locale #$(and locale - (locale-definition-source -(locale-name->definition locale) -(when locale - (format port "\ +(let* ((entry (first all-entries)) + (device (menu-entry-device entry)) + (mount-point (menu-entry-device-mount-point entry)) + (bootloader (bootloader-configuration-bootloader config)) + (grub (bootloader-package bootloader)) + (locale-dir (normalize-file (grub-locale-folder grub) + mount-point + store-directory-prefix))) + + #~(let ((locale #$(and locale + (locale-definition-source + (locale-name->definition locale + (locale-dir #$(and locale locale-dir))) + (when locale +(format port "\ # Localization configuration. -if search --file --set boot_partition /grub/grub.cfg; then -set locale_dir=(${boot_partition})/grub/locale -else -set locale_dir=/boot/grub/locale -fi -set lang=~a~%" locale +~asearch --file --set ~a/e...@quot.mo +set locale_dir=~a +set lang=~a~%" +;; Skip the search if there is an image, because it is +;; loaded before this fragment, to avoid the extra search. +#$(if (grub-theme-image (bootloader-theme config)) + "# " + "") +locale-dir +locale-dir +locale) (define keyboard-layout-config (let* ((layout (bootloader-configuration-keyboard-layout config)) -- 2.28.0
bug#44196: [PATCH 4/3] system: Fix dependency for grub.cfg generation.
This patch LGTM! pgpaO0I7e8poI.pgp Description: OpenPGP digital signature
bug#44196: [PATCH 1/3] system: Fix grub keymap with store in btrfs subvolume.
Is it possible for there to be no entries in all-entries at all? If not, LGTM! pgpZDS5G1yPnu.pgp Description: OpenPGP digital signature
bug#43985: Error logging in with non-Latin language
I have some graphics issues like "https://issues.guix.gnu.org/44301"; with Guix; The source of the problem seems to be GDM.
bug#44261: running a daemon with userns in relocateble pack breaks
Hello! As discussed on IRC, my initial advice about MS_PRIVATE was misguided. The real issue is the “rm_rf (new_root);” call, which removes the root directory and thus leaves child processes (the daemon) with nothing. The attached patch adds a test loosely based on yours and a fix for that. The fix (for the “userns” engine) is to make NEW_ROOT a tmpfs, such that upon completion, all we need to do is to unmount it and remove it; it lives on as the root file system of child processes. In the “fakechroot” case, we have to leave NEW_ROOT behind, which is not great but acceptable (it’s user-owned, #o700, and it’s under /tmp). The test only checks the “userns” engine. If you confirm that it works for you and looks reasonable, we can apply it. Thanks, Ludo’. diff --git a/gnu/packages/aux-files/run-in-namespace.c b/gnu/packages/aux-files/run-in-namespace.c index 52a16a5362..1d64ef9f44 100644 --- a/gnu/packages/aux-files/run-in-namespace.c +++ b/gnu/packages/aux-files/run-in-namespace.c @@ -41,6 +41,7 @@ #include #include #include +#include /* Whether we're building the ld.so/libfakechroot wrapper. */ #define HAVE_EXEC_WITH_LOADER \ @@ -258,11 +259,20 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) { /* Spawn @WRAPPED_PROGRAM@ in a separate namespace where STORE is bind-mounted in the right place. */ - int err; + int err, is_tmpfs; char *new_root = mkdtemp (strdup ("/tmp/guix-exec-XX")); char *new_store = concat (new_root, original_store); char *cwd = get_current_dir_name (); + /* Become the new parent of grand-children when their parent dies. */ + prctl (PR_SET_CHILD_SUBREAPER, 1); + + /* Optionally, make NEW_ROOT a tmpfs. That way, if we have to leave it + behind because there are sub-processes still running when this wrapper + exits, it's OK. */ + err = mount ("none", new_root, "tmpfs", 0, NULL); + is_tmpfs = (err == 0); + /* Create a child with separate namespaces and set up bind-mounts from there. That way, bind-mounts automatically disappear when the child exits, which simplifies cleanup for the parent. Note: clone is more @@ -300,6 +310,7 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) /* Failure: user namespaces not supported. */ fprintf (stderr, "%s: error: 'clone' failed: %m\n", argv[0]); rm_rf (new_root); + free (new_root); break; default: @@ -312,10 +323,27 @@ exec_in_user_namespace (const char *store, int argc, char *argv[]) write_id_map (child, "uid_map", getuid ()); write_id_map (child, "gid_map", getgid ()); - int status; + int status, status_other; waitpid (child, &status, 0); - chdir ("/"); /* avoid EBUSY */ - rm_rf (new_root); + + if (is_tmpfs) + { + /* NEW_ROOT lives on in child processes and we no longer need it + to exist as an empty directory in the global namespace. */ + umount (new_root); + rmdir (new_root); + } + /* Check whether there are child processes left. If there are none, + we can remove NEW_ROOT just fine. Conversely, if there are + processes left (for example because this wrapper's child forked), + we have to leave NEW_ROOT behind so that those processes can still + access their root file system (XXX). */ + else if (waitpid (-1 , &status_other, WNOHANG) == -1) + { + chdir ("/"); /* avoid EBUSY */ + rm_rf (new_root); + } + free (new_root); if (WIFEXITED (status)) @@ -490,6 +518,9 @@ exec_with_loader (const char *store, int argc, char *argv[]) setenv ("FAKECHROOT_BASE", new_root, 1); + /* Become the new parent of grand-children when their parent dies. */ + prctl (PR_SET_CHILD_SUBREAPER, 1); + pid_t child = fork (); switch (child) { @@ -507,11 +538,18 @@ exec_with_loader (const char *store, int argc, char *argv[]) default: { - int status; + int status, status_other; waitpid (child, &status, 0); - chdir ("/"); /* avoid EBUSY */ - rm_rf (new_root); - free (new_root); + + /* If there are child processes still running, leave NEW_ROOT around + so they can still access it. XXX: In that case NEW_ROOT is left + behind. */ + if (waitpid (-1 , &status_other, WNOHANG) == -1) + { + chdir ("/"); /* avoid EBUSY */ + rm_rf (new_root); + free (new_root); + } close (2); /* flushing stderr should be silent */ diff --git a/tests/guix-pack-relocatable.sh b/tests/guix-pack-relocatable.sh index a960ecd209..88cbe63b59 100644 --- a/tests/guix-pack-relocatable.sh +++ b/tests/guix-pack-relocatable.sh @@ -58,6 +58,19 @@ run_without_store () fi } +# Wait for the given file to show up. Error out if it doesn't show up in a +# timely fashion. +wait_for_file () +{ +i=0 +while ! test -f "$1" && test $i -lt 20 +do + sleep 0.3 + i=`expr $i + 1` +done +test -f "$1" +} + test_directory="`mktemp -d`" export test_directory trap 'chmod -Rf +w "$test_directory"; rm -rf "$t
bug#44196: [PATCH 4/3] system: Fix dependency for grub.cfg generation.
Danny Milosavljevic writes: > This patch LGTM! Thanks for your review, pushed as 222a630e9e. Happy hacking! Miguel signature.asc Description: PGP signature
bug#44196: [PATCH 1/3] system: Fix grub keymap with store in btrfs subvolume.
Hi Danny, Thank you a lot for taking a look into this. :-) Danny Milosavljevic writes: > Is it possible for there to be no entries in all-entries at all? This is the idiom used in the rest of the file, and that would be a bug in the calling code. The entries are the operating system generations + the entries configured through the bootloader-configuration, and that would lead to a grub.cfg without any system to boot. > If not, LGTM! Pushed as c69a1c27ee. Happy hacking! Miguel signature.asc Description: PGP signature
bug#44196: [PATCH v6 3/4] system: Do not depend on locale folder generated by
I hope this is the last one, at least for bug fixes. This version uses the right name for the autogenerated file e...@quot.mo when no image is provided. Miguel Ángel Arruga Vivas writes: > Changes w.r.t. previous version: > - Call normalize-file (the parameters were there but I lost the call). > - Only call search when there is no image configured. >From 5886bdf74bda59649b3d17b691132d9d030e0fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?= Date: Sat, 24 Oct 2020 20:36:21 +0200 Subject: [PATCH] system: Do not depend on locale folder generated by grub-install. * gnu/bootloader/grub.scm (define-module): Use (guix packages). (grub-locale-folder): New computed folder. (grub-configuration-file)[locale-config]: Use grub-locale-folder only when a locale is needed. --- gnu/bootloader/grub.scm | 56 + 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm index 5508319d3b..faf319b747 100644 --- a/gnu/bootloader/grub.scm +++ b/gnu/bootloader/grub.scm @@ -25,6 +25,7 @@ (define-module (gnu bootloader grub) #:use-module (guix build union) + #:use-module (guix packages) #:use-module (guix records) #:use-module (guix store) #:use-module (guix utils) @@ -142,6 +143,24 @@ file with the resolution provided in CONFIG." (image->png image #:width width #:height height)) (_ #f) +(define (grub-locale-folder grub) + "Generate a folder with the locales from GRUB." + (define builder +#~(begin +(use-modules (ice-9 ftw)) +(let ((locale (string-append #$grub "/share/locale")) + (out#$output)) + (mkdir out) + (chdir out) + (for-each (lambda (lang) + (let ((file (string-append locale "/" lang + "/LC_MESSAGES/grub.mo")) +(dest (string-append lang ".mo"))) +(when (file-exists? file) + (copy-file file dest +(scandir locale) + (computed-file "grub-boot-locale" builder)) + (define* (eye-candy config store-device store-mount-point #:key store-directory-prefix port) "Return a gexp that writes to PORT (a port-valued gexp) the 'grub.cfg' part @@ -404,18 +423,33 @@ menuentry ~s { #:port #~port))) (define locale-config -#~(let ((locale #$(and locale - (locale-definition-source -(locale-name->definition locale) -(when locale - (format port "\ +(let* ((entry (first all-entries)) + (device (menu-entry-device entry)) + (mount-point (menu-entry-device-mount-point entry)) + (bootloader (bootloader-configuration-bootloader config)) + (grub (bootloader-package bootloader)) + (locale-dir (normalize-file (grub-locale-folder grub) + mount-point + store-directory-prefix))) + + #~(let ((locale #$(and locale + (locale-definition-source + (locale-name->definition locale + (locale-dir #$(and locale locale-dir))) + (when locale +(format port "\ # Localization configuration. -if search --file --set boot_partition /grub/grub.cfg; then -set locale_dir=(${boot_partition})/grub/locale -else -set locale_dir=/boot/grub/locale -fi -set lang=~a~%" locale +~asearch --file --set ~a/e...@quot.mo +set locale_dir=~a +set lang=~a~%" +;; Skip the search if there is an image, because it is +;; loaded before this fragment, to avoid the extra search. +#$(if (grub-theme-image (bootloader-theme config)) + "# " + "") +locale-dir +locale-dir +locale) (define keyboard-layout-config (let* ((layout (bootloader-configuration-keyboard-layout config)) -- 2.28.0
bug#44261: running a daemon with userns in relocateble pack breaks
Ludovic Courtès writes: Hi! > As discussed on IRC, my initial advice about MS_PRIVATE was misguided. > The real issue is the “rm_rf (new_root);” call, which removes the root > directory and thus leaves child processes (the daemon) with nothing. Yes, I'm not entirely sure what I thought to see yesterday; anyway the rm_rf (new_root) is indeed the thing that makes the daemon crash. > The attached patch adds a test loosely based on yours and a fix for > that. The fix (for the “userns” engine) is to make NEW_ROOT a tmpfs, > such that upon completion, all we need to do is to unmount it and remove > it; it lives on as the root file system of child processes. > > In the “fakechroot” case, we have to leave NEW_ROOT behind, which is not > great but acceptable (it’s user-owned, #o700, and it’s under /tmp). The > test only checks the “userns” engine. Yes, I think this is acceptable. > If you confirm that it works for you and looks reasonable, we can apply > it. Yes, this works. The test and also my reproducer now work fine. Thanks a lot! Janneke -- Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com
bug#43893: [PATCH v3] maint: update-guix-package: Prevent accidentally breaking guix pull.
Hi Ludovic, Ludovic Courtès writes: > Hi again, > > How about this variant of the initial script? I think it addresses the > main issues we discussed here: > > 1. By default it doesn’t re-add the source in the store, so wrong > commit/hash issues are caught when running ‘guix build guix’. > > 2. It diagnoses dirty trees early on. It does not explicitly diagnose > missing upstream commits though, but again they’re caught when > running ‘guix build guix’. > > WDYT? > > Sorry for all the back-and-forth on what looks like a tiny issue. I do > think we’re making progress anyway! > > Thanks, > Ludo’ Reproducing as a diff over the original script for brevity: > @@ -23,12 +24,15 @@ > ;;; > ;;; Code: > -(use-modules (guix) > +(use-modules (git) > + (guix) > (guix git-download) > (guix upstream) > (guix utils) > (guix base32) > (guix build utils) > + (guix i18n) > + (guix diagnostics) > (gnu packages package-management) > (ice-9 match)) > @@ -101,7 +105,43 @@ COMMIT." >(exp > (error "'guix' package definition is not as expected" exp) > - > +(define (keep-source-in-store store source) > + "Add SOURCE to the store under the name that the 'guix' package expects." > + > + ;; Add SOURCE to the store, but this time under the real name used in the > + ;; 'origin'. This allows us to build the package without having to make a > + ;; real checkout; thus, it also works when working on a private branch. > + (reload-module > + (resolve-module '(gnu packages package-management))) > + > + (let* ((source (add-to-store store > + (origin-file-name (package-source guix)) > + #t "sha256" source)) > + (root (store-path-package-name source))) > + > +;; Add an indirect GC root for SOURCE in the current directory. > +(false-if-exception (delete-file root)) > +(symlink source root) > +(add-indirect-root store > + (string-append (getcwd) "/" root)) > + > +(info (G_ "source code kept in ~a (GC root: ~a)~%") > + source root))) > + > +(define (assert-clean-checkout repository) > + "Error out if the working directory at REPOSITORY contains local > +modifications." > + (define description > +(let ((format-options (make-describe-format-options > + #:dirty-suffix "-dirty"))) > + (describe-format (describe-workdir repository) format-options))) > + > + (when (string-suffix? "-dirty" description) > +(leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~%") > + description)) > + > + (info (G_ "updating 'guix' package to '~a'~%") description)) Unfortunately this doesn't catch the case where git has explicitly been told to '--skip-worktree' on a path or file (the original cause of this bug report). See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=43893#11. > (define (main . args) >(match args > ((commit version) > @@ -113,32 +153,20 @@ COMMIT." >(hash (query-path-hash store source)) >(location (package-definition-location)) >(old-hash (content-hash-value > - (origin-hash (package-source guix) > + (origin-hash (package-source guix) > + > + (unless (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT") > + (let ((repository (repository-open "."))) > + (assert-clean-checkout repository) > + (repository-close! repository))) > + > (edit-expression location >(update-definition commit hash > #:old-hash old-hash > #:version version)) > - ;; Re-add SOURCE to the store, but this time under the real name > used > - ;; in the 'origin'. This allows us to build the package without > - ;; having to make a real checkout; thus, it also works when working > - ;; on a private branch. > - (reload-module > - (resolve-module '(gnu packages package-management))) > - > - (let* ((source (add-to-store store > - (origin-file-name (package-source > guix)) > - #t "sha256" source)) > -(root (store-path-package-name source))) > - > - ;; Add an indirect GC root for SOURCE in the current directory. > - (false-if-exception (delete-file root)) > - (symlink source root) > - (add-indirect-root store > - (string-append (getcwd) "/" root)) > - > - (format #t "source code for commit ~a: ~a (GC root: ~a)~%" > - commit source root) > + (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_CO
bug#43850: cuirass: inconsistent SQL queries execution time.
Hello, Mathieu Othacehe writes: > Hello Chris, > >> I think Ricardo mentioned that the machine running Cuirass uses an SSD >> for the root filesystem, so moving the database there may help? > > Looks like the database was already on the SSD before my tmpfs > experiment. > > mathieu@berlin ~$ df -h > Filesystem Size Used Avail Use% Mounted on > none 95G 0 95G 0% /dev > /dev/sda1 916G 321G 549G 37% / > /dev/sdb137T 34T 2.6T 94% /gnu > tmpfs95G 8.0K 95G 1% /dev/shm > tmpfs10G 2.4G 7.7G 24% /var/lib/cuirass_tmpfs > > I don't really get why I/O pressure on /dev/sdb could impact /dev/sda. > > Thanks, > > Mathieu As an aside, running --8<---cut here---start->8--- sudo sqlite3 /var/guix/db/db.sqlite vacuum --8<---cut here---end--->8--- shaved off some 40 Mb from my large database file: -rw-r--r-- 1 root root 468889600 Oct 31 00:16 db.sqlite -rw-r--r-- 1 root root 510648320 Oct 28 23:36 db.sqlite.bak Perhaps we should run 'vacuum' when invoking 'guix gc' or at some other key places (where lots of data gets removed from the DB). There's also the auto_vacuum PRAGMA, which is not enabled currently: --8<---cut here---start->8--- sqlite3 /var/guix/db/db.sqlite 'pragma auto_vacuum' 0 --8<---cut here---end--->8--- But the later doesn't necessarily sound like a good idea: Note, however, that auto-vacuum only truncates the freelist pages from the file. Auto-vacuum does not defragment the database nor repack individual database pages the way that the VACUUM command does. In fact, because it moves pages around within the file, auto-vacuum can actually make fragmentation worse. [0] [0]: https://www.sqlite.org/pragma.html#pragma_auto_vacuum Maxim
bug#39870: python-robotframework build fails
Closing, as this was fixed by Ludovic in commit 634ce81e2a8. Thank you! Maxim