[bug#61067,v2,3/5] gnu: wine-minimal: Share phases with parent wine package.

* gnu/packages/wine.scm (wine-minimal): Share phases with parent wine package.
---
 gnu/packages/wine.scm | 31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

--
2.39.1

Am Freitag, dem 27.01.2023 um 01:08 +0000 schrieb Kaelyn Takata:
> * gnu/packages/wine.scm (wine-minimal): Share phases with parent wine
> package.
> ---
>  gnu/packages/wine.scm | 31 +++++++------------------------
>  1 file changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/gnu/packages/wine.scm b/gnu/packages/wine.scm
> index 6e43494c68..b9a8396b75 100644
> --- a/gnu/packages/wine.scm
> +++ b/gnu/packages/wine.scm
> @@ -308,30 +308,13 @@ (define-public wine-minimal
>                       (delete "gettext" "perl" "pkg-config")))
>      (inputs `())
>      (arguments
> -     `(#:validate-runpath? #f
> -       #:phases
> -       (modify-phases %standard-phases
> -         (add-after 'unpack 'patch-SHELL
> -           (lambda _
> -             (substitute* "configure"
> -               ;; configure first respects CONFIG_SHELL, clobbers
> SHELL later.
> -               (("/bin/sh")
> -                (which "bash")))))
> -         (add-after 'configure 'patch-dlopen-paths
> -           ;; Hardcode dlopened sonames to absolute paths.
> -           (lambda _
> -             (let* ((library-path (search-path-as-string->list
> -                                   (getenv "LIBRARY_PATH")))
> -                    (find-so (lambda (soname)
> -                               (search-path library-path soname))))
> -               (substitute* "include/config.h"
> -                 (("(#define SONAME_.* )\"(.*)\"" _ defso soname)
> -                  (format #f "~a\"~a\"" defso (find-so
> soname))))))))
> -       #:configure-flags
> -       (list "--without-freetype"
> -             "--without-x")
> -       ,@(strip-keyword-arguments '(#:configure-flags #:phases)
> -                                  (package-arguments wine))))))
> +     (substitute-keyword-arguments (package-arguments wine)
> +       ((#:phases phases)
> +        #~(modify-phases #$phases
> +            (delete 'wrap-executable))) ;; Don't reference Vulkan
> ICD files.
> +       ((#:configure-flags _ '())
> +        #~(list "--without-freetype"
> +                "--without-x"))))))
> 
>  (define-public wine-staging-patchset-data
>    (package
> --
Doing it this way is actually dangerous, because changes in wine get
propagated to wine-minimal even if they don't affect it (we had a
similar error in emacs blowing up the size of emacs-minimal recently).
There are two possible solutions: Bind the common phases to a variable
and use that instead of %standard-phases, or make wine inherit wine-
minimal.

Side note: QA failed :(

Cheers

------- Original Message -------
On Friday, January 27th, 2023 at 6:03 PM, Liliana Marie Prikler <liliana.prikler@gmail.com> wrote:


> 
> 
> Am Freitag, dem 27.01.2023 um 01:08 +0000 schrieb Kaelyn Takata:
> 
> > * gnu/packages/wine.scm (wine-minimal): Share phases with parent wine
> > package.
> > ---
> > gnu/packages/wine.scm | 31 +++++++------------------------
> > 1 file changed, 7 insertions(+), 24 deletions(-)
> > 
> > diff --git a/gnu/packages/wine.scm b/gnu/packages/wine.scm
> > index 6e43494c68..b9a8396b75 100644
> > --- a/gnu/packages/wine.scm
> > +++ b/gnu/packages/wine.scm
> > @@ -308,30 +308,13 @@ (define-public wine-minimal
> > (delete "gettext" "perl" "pkg-config")))
> > (inputs `()) (arguments -` (#:validate-runpath? #f
> > - #:phases
> > - (modify-phases %standard-phases
> > - (add-after 'unpack 'patch-SHELL
> > - (lambda _
> > - (substitute* "configure"
> > - ;; configure first respects CONFIG_SHELL, clobbers
> > SHELL later.
> > - (("/bin/sh")
> > - (which "bash")))))
> > - (add-after 'configure 'patch-dlopen-paths
> > - ;; Hardcode dlopened sonames to absolute paths.
> > - (lambda _
> > - (let* ((library-path (search-path-as-string->list
> > - (getenv "LIBRARY_PATH")))
> > - (find-so (lambda (soname)
> > - (search-path library-path soname))))
> > - (substitute* "include/config.h"
> > - (("(#define SONAME_.* )\"(.*)\"" _ defso soname)
> > - (format #f "~a\"~a\"" defso (find-so
> > soname))))))))
> > - #:configure-flags
> > - (list "--without-freetype"
> > - "--without-x")
> > - ,@(strip-keyword-arguments '(#:configure-flags #:phases)
> > - (package-arguments wine))))))
> > + (substitute-keyword-arguments (package-arguments wine)
> > + ((#:phases phases)
> > + #~(modify-phases #$phases
> > + (delete 'wrap-executable))) ;; Don't reference Vulkan
> > ICD files.
> > + ((#:configure-flags _ '())
> > + #~(list "--without-freetype"
> > + "--without-x"))))))
> > 
> > (define-public wine-staging-patchset-data
> > (package
> > --
> 
> Doing it this way is actually dangerous, because changes in wine get
> propagated to wine-minimal even if they don't affect it (we had a
> similar error in emacs blowing up the size of emacs-minimal recently).
> There are two possible solutions: Bind the common phases to a variable
> and use that instead of %standard-phases, or make wine inherit wine-
> minimal.

I'm not sure I'll be able to work on this today, but I am fine with either approach. I suspect flipping the packages so that wine depends on wine-minimal and adds inputs and extra phases and replaces the configure flags will be simpler from a maintenance perspective since it would keep the wine and wine-minimal versions in sync automatically.

From the comment in wine.scm, the wine-minimal package is only needed for building vkd3d to avoid a dependency loop between wine and vkd3d. (A comment on that package suggests it's the widl tool needed from wine, but wine-minimal is also an input instead of a native-input so maybe a bit more than a build-time tool is needed?)

Cheers,
Kaelyn
> 
> Side note: QA failed :(
> 
> Cheers

------- Original Message -------
On Friday, January 27th, 2023 at 6:03 PM, Liliana Marie Prikler <liliana.prikler@gmail.com> wrote:


> 
> 
> Am Freitag, dem 27.01.2023 um 01:08 +0000 schrieb Kaelyn Takata:
> 
> > * gnu/packages/wine.scm (wine-minimal): Share phases with parent wine
> > package.
> > ---
> > gnu/packages/wine.scm | 31 +++++++------------------------
> > 1 file changed, 7 insertions(+), 24 deletions(-)
> > 
> > diff --git a/gnu/packages/wine.scm b/gnu/packages/wine.scm
> > index 6e43494c68..b9a8396b75 100644
> > --- a/gnu/packages/wine.scm
> > +++ b/gnu/packages/wine.scm
> > @@ -308,30 +308,13 @@ (define-public wine-minimal
> > (delete "gettext" "perl" "pkg-config")))
> > (inputs `()) (arguments -` (#:validate-runpath? #f
> > - #:phases
> > - (modify-phases %standard-phases
> > - (add-after 'unpack 'patch-SHELL
> > - (lambda _
> > - (substitute* "configure"
> > - ;; configure first respects CONFIG_SHELL, clobbers
> > SHELL later.
> > - (("/bin/sh")
> > - (which "bash")))))
> > - (add-after 'configure 'patch-dlopen-paths
> > - ;; Hardcode dlopened sonames to absolute paths.
> > - (lambda _
> > - (let* ((library-path (search-path-as-string->list
> > - (getenv "LIBRARY_PATH")))
> > - (find-so (lambda (soname)
> > - (search-path library-path soname))))
> > - (substitute* "include/config.h"
> > - (("(#define SONAME_.* )\"(.*)\"" _ defso soname)
> > - (format #f "~a\"~a\"" defso (find-so
> > soname))))))))
> > - #:configure-flags
> > - (list "--without-freetype"
> > - "--without-x")
> > - ,@(strip-keyword-arguments '(#:configure-flags #:phases)
> > - (package-arguments wine))))))
> > + (substitute-keyword-arguments (package-arguments wine)
> > + ((#:phases phases)
> > + #~(modify-phases #$phases
> > + (delete 'wrap-executable))) ;; Don't reference Vulkan
> > ICD files.
> > + ((#:configure-flags _ '())
> > + #~(list "--without-freetype"
> > + "--without-x"))))))
> > 
> > (define-public wine-staging-patchset-data
> > (package
> > --
> 
> Doing it this way is actually dangerous, because changes in wine get
> propagated to wine-minimal even if they don't affect it (we had a
> similar error in emacs blowing up the size of emacs-minimal recently).
> There are two possible solutions: Bind the common phases to a variable
> and use that instead of %standard-phases, or make wine inherit wine-
> minimal.

I just sent a patch to this issue to make wine inherit from wine-minimal instead of vice versa. If it seems fairly reasonable, I'll rebase and reorder the patch series to include the change (essentially to fold it into the commit that updates wine-minimal to share phases with wine).

BTW, the patch also addresses a few lint messages.

Cheers,
Kaelyn

> 
> Side note: QA failed :(
> 
> Cheers

Am Montag, dem 30.01.2023 um 20:39 +0000 schrieb Kaelyn:
> ------- Original Message -------
> On Friday, January 27th, 2023 at 6:03 PM, Liliana Marie Prikler
> <liliana.prikler@gmail.com> wrote:
> 
> 
> > 
> > 
> > Am Freitag, dem 27.01.2023 um 01:08 +0000 schrieb Kaelyn Takata:
> > 
> > > * gnu/packages/wine.scm (wine-minimal): Share phases with parent
> > > wine
> > > package.
> > > ---
> > > gnu/packages/wine.scm | 31 +++++++------------------------
> > > 1 file changed, 7 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/gnu/packages/wine.scm b/gnu/packages/wine.scm
> > > index 6e43494c68..b9a8396b75 100644
> > > --- a/gnu/packages/wine.scm
> > > +++ b/gnu/packages/wine.scm
> > > @@ -308,30 +308,13 @@ (define-public wine-minimal
> > > (delete "gettext" "perl" "pkg-config")))
> > > (inputs `()) (arguments -` (#:validate-runpath? #f
> > > - #:phases
> > > - (modify-phases %standard-phases
> > > - (add-after 'unpack 'patch-SHELL
> > > - (lambda _
> > > - (substitute* "configure"
> > > - ;; configure first respects CONFIG_SHELL, clobbers
> > > SHELL later.
> > > - (("/bin/sh")
> > > - (which "bash")))))
> > > - (add-after 'configure 'patch-dlopen-paths
> > > - ;; Hardcode dlopened sonames to absolute paths.
> > > - (lambda _
> > > - (let* ((library-path (search-path-as-string->list
> > > - (getenv "LIBRARY_PATH")))
> > > - (find-so (lambda (soname)
> > > - (search-path library-path soname))))
> > > - (substitute* "include/config.h"
> > > - (("(#define SONAME_.* )\"(.*)\"" _ defso soname)
> > > - (format #f "~a\"~a\"" defso (find-so
> > > soname))))))))
> > > - #:configure-flags
> > > - (list "--without-freetype"
> > > - "--without-x")
> > > - ,@(strip-keyword-arguments '(#:configure-flags #:phases)
> > > - (package-arguments wine))))))
> > > + (substitute-keyword-arguments (package-arguments wine)
> > > + ((#:phases phases)
> > > + #~(modify-phases #$phases
> > > + (delete 'wrap-executable))) ;; Don't reference Vulkan
> > > ICD files.
> > > + ((#:configure-flags _ '())
> > > + #~(list "--without-freetype"
> > > + "--without-x"))))))
> > > 
> > > (define-public wine-staging-patchset-data
> > > (package
> > > --
> > 
> > Doing it this way is actually dangerous, because changes in wine
> > get
> > propagated to wine-minimal even if they don't affect it (we had a
> > similar error in emacs blowing up the size of emacs-minimal
> > recently).
> > There are two possible solutions: Bind the common phases to a
> > variable
> > and use that instead of %standard-phases, or make wine inherit
> > wine-
> > minimal.
> 
> I just sent a patch to this issue to make wine inherit from wine-
> minimal instead of vice versa. If it seems fairly reasonable, I'll
> rebase and reorder the patch series to include the change
> (essentially to fold it into the commit that updates wine-minimal to
> share phases with wine).
Please do.  Also, don't forget to bump the reroll count as well as
adding me to CC.

Thanks