* [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. @ 2015-07-07 8:33 Alex Kost 2015-07-07 8:43 ` Paul van der Walt 2015-07-07 15:07 ` Mark H Weaver 0 siblings, 2 replies; 8+ messages in thread From: Alex Kost @ 2015-07-07 8:33 UTC (permalink / raw) To: guix-devel [-- Attachment #1: Type: text/plain, Size: 206 bytes --] I've noticed that the packages for zathura plugins install .desktop files into: /gnu/store/…/usr/share/applications/ instead of: /gnu/store/…/share/applications/ The attached patch fixes it. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-gnu-pdf-Fix-installing-desktop-files-of-zathura-pack.patch --] [-- Type: text/x-patch, Size: 2100 bytes --] From 74efced6076ef28b321cb2a6a69344f55b714b59 Mon Sep 17 00:00:00 2001 From: Alex Kost <alezost@gmail.com> Date: Tue, 7 Jul 2015 11:17:06 +0300 Subject: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. * gnu/packages/pdf.scm (zathura-cb, zathura-ps, zathura-djvu, zathura-pdf-poppler): Add PREFIX to 'make-flags' to install ".desktop" files into "share", not "usr/share". --- gnu/packages/pdf.scm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm index 82e8c88..0e8817d 100644 --- a/gnu/packages/pdf.scm +++ b/gnu/packages/pdf.scm @@ -173,7 +173,7 @@ (arguments `(#:make-flags `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -204,7 +204,7 @@ using libarchive.") (arguments `(#:make-flags `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -236,7 +236,7 @@ using libspectre.") (arguments `(#:make-flags `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -269,7 +269,7 @@ using the DjVuLibre library.") (arguments `(#:make-flags `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") #:tests? #f ; Package does not include tests. #:phases (alist-delete 'configure %standard-phases))) -- 2.4.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 8:33 [PATCH] gnu: pdf: Fix installing desktop files of zathura packages Alex Kost @ 2015-07-07 8:43 ` Paul van der Walt 2015-07-07 17:40 ` Alex Kost 2015-07-07 15:07 ` Mark H Weaver 1 sibling, 1 reply; 8+ messages in thread From: Paul van der Walt @ 2015-07-07 8:43 UTC (permalink / raw) To: Alex Kost; +Cc: guix-devel Hey Alex, On 2015-07-07 at 10:33, quoth Alex Kost: > I've noticed that the packages for zathura plugins install .desktop > files into: > > /gnu/store/…/usr/share/applications/ instead of: > /gnu/store/…/share/applications/ > > The attached patch fixes it. My bad! Thanks for catching that. I admit that with my workflow this is something i'd never noticed / figured out why it was important, so i'm glad someone else has taken a look. Woo, zathura on Guix has more than one user now, presumably! :) Ciao, p. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 8:43 ` Paul van der Walt @ 2015-07-07 17:40 ` Alex Kost 0 siblings, 0 replies; 8+ messages in thread From: Alex Kost @ 2015-07-07 17:40 UTC (permalink / raw) To: Paul van der Walt; +Cc: guix-devel Paul van der Walt (2015-07-07 11:43 +0300) wrote: > Hey Alex, > > On 2015-07-07 at 10:33, quoth Alex Kost: >> I've noticed that the packages for zathura plugins install .desktop >> files into: >> >> /gnu/store/…/usr/share/applications/ instead of: >> /gnu/store/…/share/applications/ >> >> The attached patch fixes it. > > My bad! Thanks for catching that. I admit that with my workflow this > is something i'd never noticed / figured out why it was important, so > i'm glad someone else has taken a look. Woo, zathura on Guix has more > than one user now, presumably! :) Yeah :-) I've been using it for some time (thanks for packaging it!) and I just accidentally noticed "usr" directory in my .guix-profile. -- Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 8:33 [PATCH] gnu: pdf: Fix installing desktop files of zathura packages Alex Kost 2015-07-07 8:43 ` Paul van der Walt @ 2015-07-07 15:07 ` Mark H Weaver 2015-07-07 17:39 ` Alex Kost 1 sibling, 1 reply; 8+ messages in thread From: Mark H Weaver @ 2015-07-07 15:07 UTC (permalink / raw) To: Alex Kost; +Cc: guix-devel Alex Kost <alezost@gmail.com> writes: > I've noticed that the packages for zathura plugins install .desktop > files into: > > /gnu/store/…/usr/share/applications/ instead of: > /gnu/store/…/share/applications/ > > The attached patch fixes it. Good catch, but see below: > From 74efced6076ef28b321cb2a6a69344f55b714b59 Mon Sep 17 00:00:00 2001 > From: Alex Kost <alezost@gmail.com> > Date: Tue, 7 Jul 2015 11:17:06 +0300 > Subject: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. > > * gnu/packages/pdf.scm (zathura-cb, zathura-ps, zathura-djvu, > zathura-pdf-poppler): Add PREFIX to 'make-flags' to install > ".desktop" files into "share", not "usr/share". > --- > gnu/packages/pdf.scm | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm > index 82e8c88..0e8817d 100644 > --- a/gnu/packages/pdf.scm > +++ b/gnu/packages/pdf.scm > @@ -173,7 +173,7 @@ > (arguments > `(#:make-flags > `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) > - "PLUGINDIR=/lib/zathura" "CC=gcc") > + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") It would be better to leave DESTDIR empty and set PREFIX=<%output>, so: > - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) > + `(,(string-append "PREFIX=" (assoc-ref %outputs "out")) There is a conceptual difference between PREFIX and DESTDIR: at install time, files are copied to ${DESTDIR}${PREFIX}, and then at run time files are expected to be at ${PREFIX}. So in general, we don't want to use DESTDIR in Guix, and we want to set PREFIX to the output directory. Thanks! Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 15:07 ` Mark H Weaver @ 2015-07-07 17:39 ` Alex Kost 2015-07-07 18:36 ` Mark H Weaver 0 siblings, 1 reply; 8+ messages in thread From: Alex Kost @ 2015-07-07 17:39 UTC (permalink / raw) To: Mark H Weaver; +Cc: guix-devel Mark H Weaver (2015-07-07 18:07 +0300) wrote: [...] >> (arguments >> `(#:make-flags >> `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >> - "PLUGINDIR=/lib/zathura" "CC=gcc") >> + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") > > It would be better to leave DESTDIR empty and set PREFIX=<%output>, so: It wouldn't work for these packages. >> - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >> + `(,(string-append "PREFIX=" (assoc-ref %outputs "out")) > > There is a conceptual difference between PREFIX and DESTDIR: at install > time, files are copied to ${DESTDIR}${PREFIX}, and then at run time > files are expected to be at ${PREFIX}. So in general, we don't want to > use DESTDIR in Guix, and we want to set PREFIX to the output directory. I know, but these zathura plugins do not provide configure stages, and PREFIX is not even used in the manually written "Makefile"s. PREFIX is used in "config.mk" (which is included in a Makefile) to define LIBDIR and DESKTOPPREFIX. And due to Makefile things are installed in ${DESTDIR}${PLUGINDIR} and ${DESTDIR}${DESKTOPPREFIX}. -- Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 17:39 ` Alex Kost @ 2015-07-07 18:36 ` Mark H Weaver 2015-07-07 19:25 ` Alex Kost 0 siblings, 1 reply; 8+ messages in thread From: Mark H Weaver @ 2015-07-07 18:36 UTC (permalink / raw) To: Alex Kost; +Cc: guix-devel [-- Attachment #1: Type: text/plain, Size: 2166 bytes --] Alex Kost <alezost@gmail.com> writes: > Mark H Weaver (2015-07-07 18:07 +0300) wrote: > > [...] >>> (arguments >>> `(#:make-flags >>> `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >>> - "PLUGINDIR=/lib/zathura" "CC=gcc") >>> + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") >> >> It would be better to leave DESTDIR empty and set PREFIX=<%output>, so: > > It wouldn't work for these packages. I made this change and the packages seem to build properly. See below for my proposed patch. >>> - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >>> + `(,(string-append "PREFIX=" (assoc-ref %outputs "out")) >> >> There is a conceptual difference between PREFIX and DESTDIR: at install >> time, files are copied to ${DESTDIR}${PREFIX}, and then at run time >> files are expected to be at ${PREFIX}. So in general, we don't want to >> use DESTDIR in Guix, and we want to set PREFIX to the output directory. > > I know, but these zathura plugins do not provide configure stages, and > PREFIX is not even used in the manually written "Makefile"s. PREFIX is > used in "config.mk" (which is included in a Makefile) to define LIBDIR > and DESKTOPPREFIX. > > And due to Makefile things are installed in ${DESTDIR}${PLUGINDIR} and > ${DESTDIR}${DESKTOPPREFIX}. Okay, but the default values of PLUGINDIR and DESKTOPPREFIX are defined in terms of PREFIX. Those variables should contain the absolute pathnames of where these files will be located at run time. As is, these zathura-* packages are setting DESTDIR, PREFIX, and PLUGINDIR incorrectly. Now, it may be that in this case, it'll work properly anyway (for now), but I'd still prefer to set them correctly for two reasons: * It is very common for packagers to look at existing packages for examples of good practices, and I don't want to spread this confusion about DESTDIR and PREFIX. * Even if these build systems don't currently depend on proper settings of PREFIX, DESTDIR, and PLUGINDIR today, there's no guarantee that this will remain the case tomorrow. So, how about the following patch instead? What do you think? Mark [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: [PATCH] gnu: zathura-{cb,ps,djvu,pdf-poppler}: Fix installation of desktop files --] [-- Type: text/x-patch, Size: 2623 bytes --] From 62c2cc48e6a34c596e1e5a0bd0f873cd3d3f9218 Mon Sep 17 00:00:00 2001 From: Mark H Weaver <mhw@netris.org> Date: Tue, 7 Jul 2015 14:20:54 -0400 Subject: [PATCH] gnu: zathura-{cb,ps,djvu,pdf-poppler}: Fix installation of desktop files. Based on a patch by Alex Kost <alezost@gmail.com>. * gnu/packages/pdf.scm (zathura-cb, zathura-ps, zathura-djvu) (zathura-pdf-poppler)[arguments]: In make-flags, set PREFIX instead of DESTDIR and adjust PLUGINDIR accordingly. --- gnu/packages/pdf.scm | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm index 82e8c88..6cdc846 100644 --- a/gnu/packages/pdf.scm +++ b/gnu/packages/pdf.scm @@ -172,8 +172,9 @@ (build-system gnu-build-system) (arguments `(#:make-flags - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + `(,(string-append "PREFIX=" %output) + ,(string-append "PLUGINDIR=" %output "/lib/zathura") + "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -203,8 +204,9 @@ using libarchive.") (build-system gnu-build-system) (arguments `(#:make-flags - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + `(,(string-append "PREFIX=" %output) + ,(string-append "PLUGINDIR=" %output "/lib/zathura") + "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -235,8 +237,9 @@ using libspectre.") (build-system gnu-build-system) (arguments `(#:make-flags - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + `(,(string-append "PREFIX=" %output) + ,(string-append "PLUGINDIR=" %output "/lib/zathura") + "CC=gcc") #:tests? #f ; Package does not contain tests. #:phases (alist-delete 'configure %standard-phases))) @@ -268,8 +271,9 @@ using the DjVuLibre library.") (build-system gnu-build-system) (arguments `(#:make-flags - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) - "PLUGINDIR=/lib/zathura" "CC=gcc") + `(,(string-append "PREFIX=" %output) + ,(string-append "PLUGINDIR=" %output "/lib/zathura") + "CC=gcc") #:tests? #f ; Package does not include tests. #:phases (alist-delete 'configure %standard-phases))) -- 2.4.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 18:36 ` Mark H Weaver @ 2015-07-07 19:25 ` Alex Kost 2015-07-07 19:59 ` Mark H Weaver 0 siblings, 1 reply; 8+ messages in thread From: Alex Kost @ 2015-07-07 19:25 UTC (permalink / raw) To: Mark H Weaver; +Cc: guix-devel Mark H Weaver (2015-07-07 21:36 +0300) wrote: > Alex Kost <alezost@gmail.com> writes: > >> Mark H Weaver (2015-07-07 18:07 +0300) wrote: >> >> [...] >>>> (arguments >>>> `(#:make-flags >>>> `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >>>> - "PLUGINDIR=/lib/zathura" "CC=gcc") >>>> + "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc") >>> >>> It would be better to leave DESTDIR empty and set PREFIX=<%output>, so: >> >> It wouldn't work for these packages. > > I made this change and the packages seem to build properly. See below > for my proposed patch. It's not fair! The packages are built properly with the change of PLUGINDIR that you didn't mention. I'm joking :-) >>>> - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >>>> + `(,(string-append "PREFIX=" (assoc-ref %outputs "out")) >>> >>> There is a conceptual difference between PREFIX and DESTDIR: at install >>> time, files are copied to ${DESTDIR}${PREFIX}, and then at run time >>> files are expected to be at ${PREFIX}. So in general, we don't want to >>> use DESTDIR in Guix, and we want to set PREFIX to the output directory. >> >> I know, but these zathura plugins do not provide configure stages, and >> PREFIX is not even used in the manually written "Makefile"s. PREFIX is >> used in "config.mk" (which is included in a Makefile) to define LIBDIR >> and DESKTOPPREFIX. >> >> And due to Makefile things are installed in ${DESTDIR}${PLUGINDIR} and >> ${DESTDIR}${DESKTOPPREFIX}. > > Okay, but the default values of PLUGINDIR and DESKTOPPREFIX are defined > in terms of PREFIX. Those variables should contain the absolute > pathnames of where these files will be located at run time. > > As is, these zathura-* packages are setting DESTDIR, PREFIX, and > PLUGINDIR incorrectly. Now, it may be that in this case, it'll work > properly anyway (for now), but I'd still prefer to set them correctly > for two reasons: > > * It is very common for packagers to look at existing packages for > examples of good practices, and I don't want to spread this confusion > about DESTDIR and PREFIX. > > * Even if these build systems don't currently depend on proper settings > of PREFIX, DESTDIR, and PLUGINDIR today, there's no guarantee that > this will remain the case tomorrow. > > So, how about the following patch instead? What do you think? I absolutely agree, thanks for the detailed description! [...] > diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm > index 82e8c88..6cdc846 100644 > --- a/gnu/packages/pdf.scm > +++ b/gnu/packages/pdf.scm > @@ -172,8 +172,9 @@ > (build-system gnu-build-system) > (arguments > `(#:make-flags > - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) > - "PLUGINDIR=/lib/zathura" "CC=gcc") > + `(,(string-append "PREFIX=" %output) > + ,(string-append "PLUGINDIR=" %output "/lib/zathura") > + "CC=gcc") As you change the whole clause anyway, wouldn't it be more good-looking to remove those "`" / "," and just use "list" instead?: (list (string-append "PREFIX=" %output) (string-append "PLUGINDIR=" %output "/lib/zathura") "CC=gcc") -- Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages. 2015-07-07 19:25 ` Alex Kost @ 2015-07-07 19:59 ` Mark H Weaver 0 siblings, 0 replies; 8+ messages in thread From: Mark H Weaver @ 2015-07-07 19:59 UTC (permalink / raw) To: Alex Kost; +Cc: guix-devel Alex Kost <alezost@gmail.com> writes: > Mark H Weaver (2015-07-07 21:36 +0300) wrote: > >> diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm >> index 82e8c88..6cdc846 100644 >> --- a/gnu/packages/pdf.scm >> +++ b/gnu/packages/pdf.scm >> @@ -172,8 +172,9 @@ >> (build-system gnu-build-system) >> (arguments >> `(#:make-flags >> - `(,(string-append "DESTDIR=" (assoc-ref %outputs "out")) >> - "PLUGINDIR=/lib/zathura" "CC=gcc") >> + `(,(string-append "PREFIX=" %output) >> + ,(string-append "PLUGINDIR=" %output "/lib/zathura") >> + "CC=gcc") > > As you change the whole clause anyway, wouldn't it be more good-looking > to remove those "`" / "," and just use "list" instead?: > > (list (string-append "PREFIX=" %output) > (string-append "PLUGINDIR=" %output "/lib/zathura") > "CC=gcc") Agreed. Pushed with this change. Thanks! Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-07-07 19:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-07 8:33 [PATCH] gnu: pdf: Fix installing desktop files of zathura packages Alex Kost 2015-07-07 8:43 ` Paul van der Walt 2015-07-07 17:40 ` Alex Kost 2015-07-07 15:07 ` Mark H Weaver 2015-07-07 17:39 ` Alex Kost 2015-07-07 18:36 ` Mark H Weaver 2015-07-07 19:25 ` Alex Kost 2015-07-07 19:59 ` Mark H Weaver
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.