[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Sharlatan Hellseher]

Hi Guix,

This patch series adds support of CalcMyScky/ShowMySky module for
Stellarium that simulates scattering of light by the atmosphere.

It also include both Qt6 and Qt5 - qxlsx and calcmysky to be
compatible with Stellarium build which is packed using Qt5 inputs.

Sharlatan Hellseher (4):
  gnu: Add calcmysky.
  gnu: qxlsx: Use Qt6.
  gnu: Add qxlsx-qt5.
  gnu: stellarium: Enable ShowMySky.

 gnu/packages/astronomy.scm | 64 +++++++++++++++++++++++++++++++++++---
 gnu/packages/qt.scm        | 10 +++++-
 2 files changed, 69 insertions(+), 5 deletions(-)


base-commit: c81d2d448cbd051800867fe3f4b82ef3f4380ebf
-- 
2.39.1

[bug#61674] [PATCH 1/4] gnu: Add calcmysky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (calcmysky, calcmysky-qt5): New variables.
---
 gnu/packages/astronomy.scm | 57 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 5cee981671..f13f74a5e8 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -26,6 +26,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu packages astronomy)
+  #:use-module ((guix build utils) #:select (alist-replace))
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (gnu packages algebra)
   #:use-module (gnu packages autotools)
@@ -209,6 +210,62 @@ (define-public calceph
 @end itemize\n")
     (license license:cecill)))
 
+(define-public calcmysky
+  (package
+    (name "calcmysky")
+    (version "0.2.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/10110111/CalcMySky")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0bib5shy8wzc7j5ph218dl9hqrqip491mn25gakyghbvaqxgm27d"))))
+    (build-system cmake-build-system)
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=6")))
+    (inputs
+     (list eigen glm qtbase))
+    (home-page "https://10110111.github.io/CalcMySky/")
+    (synopsis "Simulator of light scattering by planetary atmospheres")
+    (description
+     "CalcMySky is a software package that simulates scattering of light by the
+atmosphere to render daytime and twilight skies (without stars).  Its primary
+purpose is to enable realistic view of the sky in applications such as
+planetaria.  Secondary objective is to make it possible to explore atmospheric
+effects such as glories, fogbows etc., as well as simulate unusual environments
+such as on Mars or an exoplanet orbiting a star with a non-solar spectrum of
+radiation.
+
+This package consists of three parts:
+
+@itemize
+@item @code{calcmysky} utility that does the precomputation of the atmosphere
+model to enable rendering.
+
+@item @code{libShowMySky} library that lets the applications render the
+atmosphere model.
+
+@item @code{ShowMySky} preview GUI that makes it possible to preview the
+rendering of the atmosphere model and examine its properties.
+@end itemize")
+    (license license:gpl3)))
+
+(define-public calcmysky-qt5
+  (package
+    (inherit calcmysky)
+    (name "calcmysky-qt5")
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=5")))
+    (inputs
+     (alist-replace "qtbase" (list qtbase-5)
+                    (package-inputs calcmysky)))
+    (synopsis "Qt5 build for the CalcMySky library.")))
+
 (define-public aoflagger
   (package
     (name "aoflagger")
-- 
2.39.1

[bug#61674] [PATCH 2/4] gnu: qxlsx: Use Qt6.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx):
  [inputs]: Use QTBASE (Qt6) instead QTBASE-5. Add LIBXKBCOMMON,
  VULKAN-HEADERS.
---
 gnu/packages/qt.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 76e9e519c7..4985a79db4 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1148,7 +1148,7 @@ (define-public qxlsx
                   (invoke "make" "-j" (number->string (parallel-job-count)))
                   (invoke "./TestExcel"))))))))
      (inputs
-      (list qtbase-5))
+      (list libxkbcommon qtbase vulkan-headers))
      (home-page "https://qtexcel.github.io/QXlsx/")
      (synopsis "C++ library to read/write Excel XLSX files using Qt")
      (description
-- 
2.39.1

[bug#61674] [PATCH 3/4] gnu: Add qxlsx-qt5.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx-qt5): New variable.
---
 gnu/packages/qt.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 4985a79db4..b2e14e5757 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1156,6 +1156,14 @@ (define-public qxlsx
 XLSX document format.")
      (license license:expat)))
 
+(define-public qxlsx-qt5
+  (package
+    (inherit qxlsx)
+    (name "qxlsx-qt5")
+    (inputs
+     (list qtbase-5))
+    (synopsis "Qt5 build for the qxlsx library.")))
+
 (define-public qtxmlpatterns
   (package (inherit qtsvg-5)
     (name "qtxmlpatterns")
-- 
2.39.1

[bug#61674] [PATCH 4/4] gnu: stellarium: Enable ShowMySky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (stellarium):
  [arguments]<#:configure-flags>: Enable ShowMySky optional dependencies
  to simulate scattering of light by the atmosphere.
  [inputs]: Replace QXLSX to QXLSX-QT5. Add CALCMYSKY-QT5.
---
 gnu/packages/astronomy.scm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index f13f74a5e8..e15df2874a 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -917,8 +917,6 @@ (define-public stellarium
       #~(list "-DENABLE_GPS=1"
               ;; TODO: Enable when all of the dependencies are availalbe for Qt6.
               "-DENABLE_QT6=0"
-              ;; TODO: Pack missing in Guix https://10110111.github.io/CalcMySky/
-              "-DENABLE_SHOWMYSKY=0"
               "-DENABLE_TESTING=0"
               (string-append "-DCMAKE_CXX_FLAGS=-isystem "
                              #$(this-package-input "qtserialport") "/include/qt5"))
@@ -929,7 +927,8 @@ (define-public stellarium
               (setenv "QT_QPA_PLATFORM" "offscreen")
               (setenv "HOME" "/tmp"))))))
     (inputs
-     (list gpsd
+     (list calcmysky-qt5
+           gpsd
            indi
            libnova
            openssl
@@ -942,7 +941,7 @@ (define-public stellarium
            qtserialport
            qttranslations
            qtwebengine-5
-           qxlsx
+           qxlsx-qt5
            zlib))
     (native-inputs
      (list doxygen
-- 
2.39.1

[bug#61674]

[Sharlatan Hellseher]

Hi Guix!

I've found the fix for aarch64-linux, thanks to the author's replay -
https://github.com/10110111/CalcMySky/issues/12
v2 should pass the QA now :)

Thanks.

-- 
… наш разум - превосходная объяснительная машина которая способна
найти смысл почти в чем угодно, истолковать любой феномен, но
совершенно не в состоянии принять мысль о непредсказуемости.

[bug#61674] [PATCH 1/4] gnu: Add calcmysky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (calcmysky, calcmysky-qt5): New variables.
---
 gnu/packages/astronomy.scm | 59 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 5cee981671..0f3176bc3e 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -26,6 +26,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu packages astronomy)
+  #:use-module ((guix build utils) #:select (alist-replace))
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (gnu packages algebra)
   #:use-module (gnu packages autotools)
@@ -209,6 +210,64 @@ (define-public calceph
 @end itemize\n")
     (license license:cecill)))
 
+(define-public calcmysky
+  (package
+    (name "calcmysky")
+    (version "0.2.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/10110111/CalcMySky")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0bib5shy8wzc7j5ph218dl9hqrqip491mn25gakyghbvaqxgm27d"))))
+    (build-system cmake-build-system)
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=6"
+                   "-DCMAKE_CXX_FLAGS=-fPIC")))
+    (inputs
+     (list eigen glm qtbase))
+    (home-page "https://10110111.github.io/CalcMySky/")
+    (synopsis "Simulator of light scattering by planetary atmospheres")
+    (description
+     "CalcMySky is a software package that simulates scattering of light by the
+atmosphere to render daytime and twilight skies (without stars).  Its primary
+purpose is to enable realistic view of the sky in applications such as
+planetaria.  Secondary objective is to make it possible to explore atmospheric
+effects such as glories, fogbows etc., as well as simulate unusual environments
+such as on Mars or an exoplanet orbiting a star with a non-solar spectrum of
+radiation.
+
+This package consists of three parts:
+
+@itemize
+@item @code{calcmysky} utility that does the precomputation of the atmosphere
+model to enable rendering.
+
+@item @code{libShowMySky} library that lets the applications render the
+atmosphere model.
+
+@item @code{ShowMySky} preview GUI that makes it possible to preview the
+rendering of the atmosphere model and examine its properties.
+@end itemize")
+    (license license:gpl3)))
+
+(define-public calcmysky-qt5
+  (package
+    (inherit calcmysky)
+    (name "calcmysky-qt5")
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=5"
+                   "-DCMAKE_CXX_FLAGS=-fPIC")))
+    (inputs
+     (alist-replace "qtbase" (list qtbase-5)
+                    (package-inputs calcmysky)))
+    (synopsis "Qt5 build for the CalcMySky library.")))
+
 (define-public aoflagger
   (package
     (name "aoflagger")
-- 
2.39.1

[bug#61674] [PATCH 2/4] gnu: qxlsx: Use Qt6.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx):
  [inputs]: Use QTBASE (Qt6) instead QTBASE-5. Add LIBXKBCOMMON,
  VULKAN-HEADERS.
---
 gnu/packages/qt.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 76e9e519c7..4985a79db4 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1148,7 +1148,7 @@ (define-public qxlsx
                   (invoke "make" "-j" (number->string (parallel-job-count)))
                   (invoke "./TestExcel"))))))))
      (inputs
-      (list qtbase-5))
+      (list libxkbcommon qtbase vulkan-headers))
      (home-page "https://qtexcel.github.io/QXlsx/")
      (synopsis "C++ library to read/write Excel XLSX files using Qt")
      (description
-- 
2.39.1

[bug#61674] [PATCH 3/4] gnu: Add qxlsx-qt5.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx-qt5): New variable.
---
 gnu/packages/qt.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 4985a79db4..b2e14e5757 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1156,6 +1156,14 @@ (define-public qxlsx
 XLSX document format.")
      (license license:expat)))
 
+(define-public qxlsx-qt5
+  (package
+    (inherit qxlsx)
+    (name "qxlsx-qt5")
+    (inputs
+     (list qtbase-5))
+    (synopsis "Qt5 build for the qxlsx library.")))
+
 (define-public qtxmlpatterns
   (package (inherit qtsvg-5)
     (name "qtxmlpatterns")
-- 
2.39.1

[bug#61674] [PATCH 4/4] gnu: stellarium: Enable ShowMySky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (stellarium):
  [arguments]<#:configure-flags>: Enable ShowMySky optional dependencies
  to simulate scattering of light by the atmosphere.
  [inputs]: Replace QXLSX to QXLSX-QT5. Add CALCMYSKY-QT5.
---
 gnu/packages/astronomy.scm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 0f3176bc3e..ffce19e8fc 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -919,8 +919,6 @@ (define-public stellarium
       #~(list "-DENABLE_GPS=1"
               ;; TODO: Enable when all of the dependencies are availalbe for Qt6.
               "-DENABLE_QT6=0"
-              ;; TODO: Pack missing in Guix https://10110111.github.io/CalcMySky/
-              "-DENABLE_SHOWMYSKY=0"
               "-DENABLE_TESTING=0"
               (string-append "-DCMAKE_CXX_FLAGS=-isystem "
                              #$(this-package-input "qtserialport") "/include/qt5"))
@@ -931,7 +929,8 @@ (define-public stellarium
               (setenv "QT_QPA_PLATFORM" "offscreen")
               (setenv "HOME" "/tmp"))))))
     (inputs
-     (list gpsd
+     (list calcmysky-qt5
+           gpsd
            indi
            libnova
            openssl
@@ -944,7 +943,7 @@ (define-public stellarium
            qtserialport
            qttranslations
            qtwebengine-5
-           qxlsx
+           qxlsx-qt5
            zlib))
     (native-inputs
      (list doxygen
-- 
2.39.1

[bug#61674] [PATCH 1/4] gnu: Add calcmysky.

[Leo Famulari]

On Sun, Feb 26, 2023 at 12:44:03AM +0000, Sharlatan Hellseher wrote:
> * gnu/packages/astronomy.scm (calcmysky, calcmysky-qt5): New variables.

Thanks for these patches!

> +  #:use-module ((guix build utils) #:select (alist-replace))
[...]
> +(define-public calcmysky-qt5
> +  (package
> +    (inherit calcmysky)
> +    (name "calcmysky-qt5")
> +    (arguments
> +     (list #:configure-flags
> +           #~(list "-DQT_VERSION=5"
> +                   "-DCMAKE_CXX_FLAGS=-fPIC")))
> +    (inputs
> +     (alist-replace "qtbase" (list qtbase-5)
> +                    (package-inputs calcmysky)))

Can you try rewriting this using modify-inputs, rather than
alist-replace?

https://guix.gnu.org/manual/devel/en/html_node/Defining-Package-Variants.html#Defining-Package-Variants

Feel free to ask for help if you get stuck.

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Ludovic Courtès]

Hi!

Thanks for a nice patch series!

Sharlatan Hellseher <sharlatanus@gmail.com> skribis:

> * gnu/packages/astronomy.scm (calcmysky, calcmysky-qt5): New variables.

[...]

> +    (license license:gpl3)))

Please double-check whether something explicitly says “version 3 only”;
if not, it’s ‘gpl3+’.

> +(define-public calcmysky-qt5
> +  (package
> +    (inherit calcmysky)

Rather:

  (package/inherit calcmysky
    (name "calcmysky-qt5")
    …)

> +    (inputs
> +     (alist-replace "qtbase" (list qtbase-5)
> +                    (package-inputs calcmysky)))

Instead of ‘alist-replace’, write

  (modify-inputs (package-inputs calcmysky)
    (replace "qtbase" qtbase-5))

You can remove the corresponding #:use-module form at the top.

Ludo’.

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Ludovic Courtès]

Sharlatan Hellseher <sharlatanus@gmail.com> skribis:

> * gnu/packages/qt.scm (qxlsx-qt5): New variable.
> ---
>  gnu/packages/qt.scm | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
> index 4985a79db4..b2e14e5757 100644
> --- a/gnu/packages/qt.scm
> +++ b/gnu/packages/qt.scm
> @@ -1156,6 +1156,14 @@ (define-public qxlsx
>  XLSX document format.")
>       (license license:expat)))
>  
> +(define-public qxlsx-qt5
> +  (package
> +    (inherit qxlsx)

Please use ‘package/inherit’ as well.

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Ludovic Courtès]

Sharlatan Hellseher <sharlatanus@gmail.com> skribis:

> * gnu/packages/astronomy.scm (stellarium):
>   [arguments]<#:configure-flags>: Enable ShowMySky optional dependencies
>   to simulate scattering of light by the atmosphere.
>   [inputs]: Replace QXLSX to QXLSX-QT5. Add CALCMYSKY-QT5.

LGTM!

Could you send an updated revision of this patch series taking into
account previous comments?  Then we should be all set!

Thanks,
Ludo’.

[bug#61674] [PATCH v3 1/4] gnu: Add calcmysky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (calcmysky, calcmysky-qt5): New variables.
---
 gnu/packages/astronomy.scm | 57 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 5cee981671..bf022c1804 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -209,6 +209,63 @@ (define-public calceph
 @end itemize\n")
     (license license:cecill)))
 
+(define-public calcmysky
+  (package
+    (name "calcmysky")
+    (version "0.2.1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/10110111/CalcMySky")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0bib5shy8wzc7j5ph218dl9hqrqip491mn25gakyghbvaqxgm27d"))))
+    (build-system cmake-build-system)
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=6"
+                   "-DCMAKE_CXX_FLAGS=-fPIC")))
+    (inputs
+     (list eigen glm qtbase))
+    (home-page "https://10110111.github.io/CalcMySky/")
+    (synopsis "Simulator of light scattering by planetary atmospheres")
+    (description
+     "CalcMySky is a software package that simulates scattering of light by the
+atmosphere to render daytime and twilight skies (without stars).  Its primary
+purpose is to enable realistic view of the sky in applications such as
+planetaria.  Secondary objective is to make it possible to explore atmospheric
+effects such as glories, fogbows etc., as well as simulate unusual environments
+such as on Mars or an exoplanet orbiting a star with a non-solar spectrum of
+radiation.
+
+This package consists of three parts:
+
+@itemize
+@item @code{calcmysky} utility that does the precomputation of the atmosphere
+model to enable rendering.
+
+@item @code{libShowMySky} library that lets the applications render the
+atmosphere model.
+
+@item @code{ShowMySky} preview GUI that makes it possible to preview the
+rendering of the atmosphere model and examine its properties.
+@end itemize")
+    (license license:gpl3+)))
+
+(define-public calcmysky-qt5
+  (package/inherit calcmysky
+    (name "calcmysky-qt5")
+    (arguments
+     (list #:configure-flags
+           #~(list "-DQT_VERSION=5"
+                   "-DCMAKE_CXX_FLAGS=-fPIC")))
+    (inputs
+     (modify-inputs (package-inputs calcmysky)
+       (replace "qtbase" qtbase-5)))
+    (synopsis "Qt5 build for the CalcMySky library.")))
+
 (define-public aoflagger
   (package
     (name "aoflagger")
-- 
2.39.1

[bug#61674] [PATCH v3 2/4] gnu: qxlsx: Use Qt6.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx):
  [inputs]: Use QTBASE (Qt6) instead QTBASE-5. Add LIBXKBCOMMON,
  VULKAN-HEADERS.
---
 gnu/packages/qt.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 76e9e519c7..4985a79db4 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1148,7 +1148,7 @@ (define-public qxlsx
                   (invoke "make" "-j" (number->string (parallel-job-count)))
                   (invoke "./TestExcel"))))))))
      (inputs
-      (list qtbase-5))
+      (list libxkbcommon qtbase vulkan-headers))
      (home-page "https://qtexcel.github.io/QXlsx/")
      (synopsis "C++ library to read/write Excel XLSX files using Qt")
      (description
-- 
2.39.1

[bug#61674] [PATCH v3 3/4] gnu: Add qxlsx-qt5.

[Sharlatan Hellseher]

* gnu/packages/qt.scm (qxlsx-qt5): New variable.
---
 gnu/packages/qt.scm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
index 4985a79db4..643744ee7a 100644
--- a/gnu/packages/qt.scm
+++ b/gnu/packages/qt.scm
@@ -1156,6 +1156,13 @@ (define-public qxlsx
 XLSX document format.")
      (license license:expat)))
 
+(define-public qxlsx-qt5
+  (package/inherit qxlsx
+    (name "qxlsx-qt5")
+    (inputs
+     (list qtbase-5))
+    (synopsis "Qt5 build for the qxlsx library.")))
+
 (define-public qtxmlpatterns
   (package (inherit qtsvg-5)
     (name "qtxmlpatterns")
-- 
2.39.1

[bug#61674] [PATCH v3 4/4] gnu: stellarium: Enable ShowMySky.

[Sharlatan Hellseher]

* gnu/packages/astronomy.scm (stellarium):
  [arguments]<#:configure-flags>: Enable ShowMySky optional dependencies
  to simulate scattering of light by the atmosphere.
  [inputs]: Replace QXLSX to QXLSX-QT5. Add CALCMYSKY-QT5.
---
 gnu/packages/astronomy.scm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index bf022c1804..473e7ce63a 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -917,8 +917,6 @@ (define-public stellarium
       #~(list "-DENABLE_GPS=1"
               ;; TODO: Enable when all of the dependencies are availalbe for Qt6.
               "-DENABLE_QT6=0"
-              ;; TODO: Pack missing in Guix https://10110111.github.io/CalcMySky/
-              "-DENABLE_SHOWMYSKY=0"
               "-DENABLE_TESTING=0"
               (string-append "-DCMAKE_CXX_FLAGS=-isystem "
                              #$(this-package-input "qtserialport") "/include/qt5"))
@@ -929,7 +927,8 @@ (define-public stellarium
               (setenv "QT_QPA_PLATFORM" "offscreen")
               (setenv "HOME" "/tmp"))))))
     (inputs
-     (list gpsd
+     (list calcmysky-qt5
+           gpsd
            indi
            libnova
            openssl
@@ -942,7 +941,7 @@ (define-public stellarium
            qtserialport
            qttranslations
            qtwebengine-5
-           qxlsx
+           qxlsx-qt5
            zlib))
     (native-inputs
      (list doxygen
-- 
2.39.1

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Simon Tournier]

Hi Ludo,

On lun., 27 févr. 2023 at 22:14, Ludovic Courtès <ludo@gnu.org> wrote:

>> +(define-public calcmysky-qt5
>> +  (package
>> +    (inherit calcmysky)
>
> Rather:
>
>   (package/inherit calcmysky
>     (name "calcmysky-qt5")
>     …)

Out of curiosity, what is the rationale for this suggestion?

Cheers,
simon

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Ludovic Courtès]

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> On lun., 27 févr. 2023 at 22:14, Ludovic Courtès <ludo@gnu.org> wrote:
>
>>> +(define-public calcmysky-qt5
>>> +  (package
>>> +    (inherit calcmysky)
>>
>> Rather:
>>
>>   (package/inherit calcmysky
>>     (name "calcmysky-qt5")
>>     …)
>
> Out of curiosity, what is the rationale for this suggestion?

This is so that the ‘replacement’ field of ‘calcmysky’, when there is
one, inherits the same transformations in ‘calcmysky-qt5’.  Quoth the
‘package/inherit’ docstring:

  Like (package (inherit P) OVERRIDES ...), except that the same
  transformation is done to the package P's replacement, if any.  P must
  be a bare identifier, and will be bound to either P or its replacement
  when evaluating OVERRIDES.

Ludo’.

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Simon Tournier]

Hi,

On Fri, 3 Mar 2023 at 11:49, Ludovic Courtès <ludo@gnu.org> wrote:

> This is so that the ‘replacement’ field of ‘calcmysky’, when there is
> one, inherits the same transformations in ‘calcmysky-qt5’.  Quoth the
> ‘package/inherit’ docstring:
>
>   Like (package (inherit P) OVERRIDES ...), except that the same
>   transformation is done to the package P's replacement, if any.  P must
>   be a bare identifier, and will be bound to either P or its replacement
>   when evaluating OVERRIDES.

Thanks for the explanations but it is still unclear.  Sorry to be slow.

It is not clear for me why you choose one over the other.  From my
current understanding, I would be tempted to always use
'package/inherit' and never plain 'inherit'.

Cheers,
simon

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Maxim Cournoyer]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On Fri, 3 Mar 2023 at 11:49, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> This is so that the ‘replacement’ field of ‘calcmysky’, when there is
>> one, inherits the same transformations in ‘calcmysky-qt5’.  Quoth the
>> ‘package/inherit’ docstring:
>>
>>   Like (package (inherit P) OVERRIDES ...), except that the same
>>   transformation is done to the package P's replacement, if any.  P must
>>   be a bare identifier, and will be bound to either P or its replacement
>>   when evaluating OVERRIDES.
>
> Thanks for the explanations but it is still unclear.  Sorry to be slow.
>
> It is not clear for me why you choose one over the other.  From my
> current understanding, I would be tempted to always use
> 'package/inherit' and never plain 'inherit'.

I also got confused by that in the past;  The way I process it
internally now is this:

If the inheritance is for *same-source/same-version* variants of a
package, they should use package/inherit, as any security issues found
in the parent package should also be applied to that package (since they
use the same source).  Otherwise, plain 'inherit' should be used
(e.g. for newer version variants).

I hope that helps!

Yours in slowness,

-- 
Maxim

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Simon Tournier]

Hi Maxim,

On Fri, 3 Mar 2023 at 16:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> If the inheritance is for *same-source/same-version* variants of a
> package, they should use package/inherit, as any security issues found
> in the parent package should also be applied to that package (since they
> use the same source).  Otherwise, plain 'inherit' should be used
> (e.g. for newer version variants).

Aahh, that makes sense. :-)  Thank you.

For instance, does it mean that

--8<---------------cut here---------------start------------->8---
(define-public gst-plugins-good-qt
  (package
    (inherit gst-plugins-good)
    (name "gst-plugins-good-qt")
    (inputs
     (modify-inputs (package-inputs gst-plugins-good)
       (prepend qtbase-5
                qtdeclarative-5
                qtwayland-5
                qtx11extras)))))
--8<---------------cut here---------------end--------------->8---

would be incorrect?  It should be 'package/inherit', right?

Cheers,
simon

The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)

[Tobias Geerinckx-Rice]

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

Hi!

Maxim Cournoyer 写道：
> Simon Tournier <zimon.toutoune@gmail.com> writes:
>> It is not clear for me why you choose one over the other.  From 
>> my
>> current understanding, I would be tempted to always use
>> 'package/inherit' and never plain 'inherit'.
>
> I also got confused by that in the past;

Same.  I think it's a rite of passage.  A questionable one.

> The way I process it internally now is this:
>
> If the inheritance is for *same-source/same-version* variants of 
> a
> package, they should use package/inherit, as any security issues 
> found
> in the parent package should also be applied to that package 
> (since they
> use the same source).  Otherwise, plain 'inherit' should be used
> (e.g. for newer version variants).

That about jives with my intuition.

Judging by the (IMO) universal confusion this causes, it is (IMO) 
spectacularly poorly-named.  A docstring doesn't fix that.

Could we rename it to something like 
‘package+replacements/inherit’?  To me, that captures the spirit, 
without being overly longer.

But I'll gladly judge other bikesheds if they lead to a less 
misleading name.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Maxim Cournoyer]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> On Fri, 3 Mar 2023 at 16:54, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>> If the inheritance is for *same-source/same-version* variants of a
>> package, they should use package/inherit, as any security issues found
>> in the parent package should also be applied to that package (since they
>> use the same source).  Otherwise, plain 'inherit' should be used
>> (e.g. for newer version variants).
>
> Aahh, that makes sense. :-)  Thank you.
>
> For instance, does it mean that
>
> (define-public gst-plugins-good-qt
>   (package
>     (inherit gst-plugins-good)
>     (name "gst-plugins-good-qt")
>     (inputs
>      (modify-inputs (package-inputs gst-plugins-good)
>        (prepend qtbase-5
>                 qtdeclarative-5
>                 qtwayland-5
>                 qtx11extras)))))
>
> would be incorrect?  It should be 'package/inherit', right?

It should be package/inherit yes, since they share the same source, thus
the same defects, thus should receive the same replacements/grafts (if
my preceding reasoning is correct :-)).

-- 
Thanks,
Maxim

Re: The package/inherit trap

[Maxim Cournoyer]

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Hi!
>
> Maxim Cournoyer 写道：
>> Simon Tournier <zimon.toutoune@gmail.com> writes:
>>> It is not clear for me why you choose one over the other.  From my
>>> current understanding, I would be tempted to always use
>>> 'package/inherit' and never plain 'inherit'.
>>
>> I also got confused by that in the past;
>
> Same.  I think it's a rite of passage.  A questionable one.
>
>> The way I process it internally now is this:
>>
>> If the inheritance is for *same-source/same-version* variants of a
>> package, they should use package/inherit, as any security issues
>> found
>> in the parent package should also be applied to that package (since
>> they
>> use the same source).  Otherwise, plain 'inherit' should be used
>> (e.g. for newer version variants).
>
> That about jives with my intuition.
>
> Judging by the (IMO) universal confusion this causes, it is (IMO)
> spectacularly poorly-named.  A docstring doesn't fix that.
>
> Could we rename it to something like ‘package+replacements/inherit’?
> To me, that captures the spirit, without being overly longer.

That'd be better, I think.  The more verbose name at least wouldn't let
one think, 'oh, some questionable syntax sugar for the lazy', like I did
in the past :-).

-- 
Thanks,
Maxim

[bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Sharlatan Hellseher]

[-- Attachment #1: Type: text/plain, Size: 1424 bytes --]

Hi all,

It was very insightful patch series I ever sent :-)
I hope I covered all recommendations in v3.

Thanks,
Oleg

On Sat, 4 Mar 2023, 03:32 Maxim Cournoyer, <maxim.cournoyer@gmail.com>
wrote:

> Hi Simon,
>
> Simon Tournier <zimon.toutoune@gmail.com> writes:
>
> > Hi Maxim,
> >
> > On Fri, 3 Mar 2023 at 16:54, Maxim Cournoyer <maxim.cournoyer@gmail.com>
> wrote:
> >
> >> If the inheritance is for *same-source/same-version* variants of a
> >> package, they should use package/inherit, as any security issues found
> >> in the parent package should also be applied to that package (since they
> >> use the same source).  Otherwise, plain 'inherit' should be used
> >> (e.g. for newer version variants).
> >
> > Aahh, that makes sense. :-)  Thank you.
> >
> > For instance, does it mean that
> >
> > (define-public gst-plugins-good-qt
> >   (package
> >     (inherit gst-plugins-good)
> >     (name "gst-plugins-good-qt")
> >     (inputs
> >      (modify-inputs (package-inputs gst-plugins-good)
> >        (prepend qtbase-5
> >                 qtdeclarative-5
> >                 qtwayland-5
> >                 qtx11extras)))))
> >
> > would be incorrect?  It should be 'package/inherit', right?
>
> It should be package/inherit yes, since they share the same source, thus
> the same defects, thus should receive the same replacements/grafts (if
> my preceding reasoning is correct :-)).
>
> --
> Thanks,
> Maxim
>

[-- Attachment #2: Type: text/html, Size: 2278 bytes --]

bug#61674: [PATCH 0/4] gnu: stellarium: Enable ShowMySky.

[Ludovic Courtès]

Hi Oleg,

Applied, thanks!

Ludo’.

Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)

[Simon Tournier]

Hi,

On Fri, 03 Mar 2023 at 20:21, Tobias Geerinckx-Rice <me@tobias.gr> wrote:

> Could we rename it to something like 
> ‘package+replacements/inherit’?  To me, that captures the spirit, 
> without being overly longer.

Well, I gave a look at the code and have seen the replacement.  But I
had not thought about the package transformation and the like.

From my point of view, the best would to add a paragraph with index
entries under “Defining-Package-Variants” section [1].

However, in the light of Maxim’s explanations, the example from the
manual appears to me inconsistent:

--8<---------------cut here---------------start------------->8---
You can just as well define variants with a different set of
dependencies than the original package.  For example, the default
@code{gdb} package depends on @code{guile}, but since that is an
optional dependency, you can define a variant that removes that
dependency like so:

@lisp
(use-modules (gnu packages gdb))   ;for 'gdb'

(define gdb-sans-guile
  (package
    (inherit gdb)
    (inputs (modify-inputs (package-inputs gdb)
              (delete "guile")))))
@end lisp
--8<---------------cut here---------------end--------------->8---

Well, since the trap is not completely clear for me yet, I am not able
to propose a paragraph.  IMHO, a paragraph here would help in mitigating
the trap.  Whatever the name of ’package/inherit’. :-)

1: <https://guix.gnu.org/manual/devel/en/guix.html#Defining-Package-Variants>


Cheers,
simon

Re: The package/inherit trap

[Maxim Cournoyer]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On Fri, 03 Mar 2023 at 20:21, Tobias Geerinckx-Rice <me@tobias.gr> wrote:
>
>> Could we rename it to something like 
>> ‘package+replacements/inherit’?  To me, that captures the spirit, 
>> without being overly longer.
>
> Well, I gave a look at the code and have seen the replacement.  But I
> had not thought about the package transformation and the like.
>
> From my point of view, the best would to add a paragraph with index
> entries under “Defining-Package-Variants” section [1].
>
> However, in the light of Maxim’s explanations, the example from the
> manual appears to me inconsistent:
>
> You can just as well define variants with a different set of
> dependencies than the original package.  For example, the default
> @code{gdb} package depends on @code{guile}, but since that is an
> optional dependency, you can define a variant that removes that
> dependency like so:
>
> @lisp
> (use-modules (gnu packages gdb))   ;for 'gdb'
>
> (define gdb-sans-guile
>   (package
>     (inherit gdb)
>     (inputs (modify-inputs (package-inputs gdb)
>               (delete "guile")))))
> @end lisp

Do you mean inconsistent because based on what I wrote it should have
used "package/inherit gdb ..." instead of (package (inherit gdb) ...) ?
If so, I agree.  It could be modified to use the former and an extra
explanation offered about why package/inherit is used here when it's to
be preferred to plain inheritance.

-- 
Thanks,
Maxim

Re: The package/inherit trap

[Simon Tournier]

Hi Maxim,

On Tue, 07 Mar 2023 at 11:43, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

>> @lisp
>> (use-modules (gnu packages gdb))   ;for 'gdb'
>>
>> (define gdb-sans-guile
>>   (package
>>     (inherit gdb)
>>     (inputs (modify-inputs (package-inputs gdb)
>>               (delete "guile")))))
>> @end lisp
>
> Do you mean inconsistent because based on what I wrote it should have
> used "package/inherit gdb ..." instead of (package (inherit gdb) ...) ?

Based on my understanding about what you wrote.

> If so, I agree.  It could be modified to use the former and an extra
> explanation offered about why package/inherit is used here when it's to
> be preferred to plain inheritance.

Well, from my point of view, we have a trap because the documentation is
not clear. :-)

Well, I think it is not only by replacing in the example.  I think the
manual should provide 2 examples and makes a clear line when one needs
to pick ’inherit’ or when one needs to pick ’package/inherit’.

Somehow, we have a similar issue as we had before with “Snippet vs
Phases“.  It would help to have plain words for ’package/inherit’ use
cases; assuming all the other use cases are covered by ’inherit’. ;-)

Cheers,
simon

Re: The package/inherit trap

[Tobias Geerinckx-Rice]

Hi,

On 7 March 2023 17:46:50 UTC, Simon Tournier <zimon.toutoune@gmail.com> wrote:
>Well, from my point of view, we have a trap because the documentation is
>not clear. :-)

Both.

However, merely documenting something is not enough when we have the chance to fix misleading naming, as we do here.  It would be nice to have, but orthogonal.



Kind regards,

T G-R

Sent on the go.  Excuse or enjoy my brevity.

Re: The package/inherit trap

[Simon Tournier]

Hi Tobias,

On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice <me@tobias.gr> wrote:

> However, merely documenting something is not enough when we have the
> chance to fix misleading naming, as we do here.  It would be nice to
> have, but orthogonal. 

I would not say it is orthogonal because renaming would not have been
enough, at least for me, in order to get the difference with ’inherit’.

For sure, it is a real trap. :-)  For instance,

    $ git log --grep='package/inherit' --oneline | grep use
    5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit.
    6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'.
    5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'.
    dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'.
    2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on replacement package.
    4163b6d855 gnu: avahi: Don't use package/inherit.

and in the light of this discussion, 5f83dd03a2 seems incorrect, no?

Well, I agree that naming is important and probably the most difficult
task. :-)

Do we go for renaming + deprecated?  Then do we apply the rename to all
occurrences in Guix?

Cheers,
simon

Re: The package/inherit trap

[John Kehayias]

Hi all,

On Wed, Mar 08, 2023 at 10:07 AM, Simon Tournier wrote:

> Hi Tobias,
>
> On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice <me@tobias.gr> wrote:
>
>> However, merely documenting something is not enough when we have the
>> chance to fix misleading naming, as we do here.  It would be nice to
>> have, but orthogonal.
>
> I would not say it is orthogonal because renaming would not have been
> enough, at least for me, in order to get the difference with ’inherit’.
>
> For sure, it is a real trap. :-)  For instance,
>
>     $ git log --grep='package/inherit' --oneline | grep use
>     5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit.
>     6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'.
>     5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'.
>     dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'.
>     2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on replacement package.
>     4163b6d855 gnu: avahi: Don't use package/inherit.
>
> and in the light of this discussion, 5f83dd03a2 seems incorrect, no?
>
> Well, I agree that naming is important and probably the most difficult
> task. :-)
>
> Do we go for renaming + deprecated?  Then do we apply the rename to all
> occurrences in Guix?
>
> Cheers,
> simon

I know, reviving an old thread but I assume this wasn't resolved?
Throw my vote for the more explicit renaming with better doc/examples,
as one who never remembers the difference either.

John

Re: The package/inherit trap

[Mark H Weaver]

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi Tobias,
>
> On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice <me@tobias.gr> wrote:
>
>> However, merely documenting something is not enough when we have the
>> chance to fix misleading naming, as we do here.  It would be nice to
>> have, but orthogonal. 
>
> I would not say it is orthogonal because renaming would not have been
> enough, at least for me, in order to get the difference with ’inherit’.
>
> For sure, it is a real trap. :-)  For instance,
>
>     $ git log --grep='package/inherit' --oneline | grep use
>     5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit.
>     6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'.
>     5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'.
>     dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'.
>     2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on replacement package.
>     4163b6d855 gnu: avahi: Don't use package/inherit.
>
> and in the light of this discussion, 5f83dd03a2 seems incorrect, no?

I agree that commit 5f83dd03a2 is incorrect.  'kodi/wayland' is quite
clearly a case where 'package/inherit' should be used.

       Mark

Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)

[(]

Tobias Geerinckx-Rice <me@tobias.gr> writes:
> But I'll gladly judge other bikesheds if they lead to a less misleading name.

How about 'package/variant' or 'package/variant-of'?

  -- (

Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)

[Attila Lendvai]

> How about 'package/variant' or 'package/variant-of'?

+1 for trying to capture the intention in the name, instead of the means of implementation.

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“Although teachers do care and do work very, very hard, the institution is psychopathic-it has no conscience. It rings a bell and the young man in the middle of writing a poem must close his notebook and move to a different cell where he must memorize that humans and monkeys derive from a common ancestor.”
	— John Taylor Gatto (1935–2018), 'Dumbing Us Down: The Hidden Curriculum of Compulsory Education' (1992)

Re: The package/inherit trap

[Maxim Cournoyer]

Hello,

Mark H Weaver <mhw@netris.org> writes:

> Simon Tournier <zimon.toutoune@gmail.com> writes:
>
>> Hi Tobias,
>>
>> On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice <me@tobias.gr> wrote:
>>
>>> However, merely documenting something is not enough when we have the
>>> chance to fix misleading naming, as we do here.  It would be nice to
>>> have, but orthogonal. 
>>
>> I would not say it is orthogonal because renaming would not have been
>> enough, at least for me, in order to get the difference with ’inherit’.
>>
>> For sure, it is a real trap. :-)  For instance,
>>
>>     $ git log --grep='package/inherit' --oneline | grep use
>>     5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit.
>>     6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'.
>>     5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'.
>>     dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'.
>>     2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on replacement package.
>>     4163b6d855 gnu: avahi: Don't use package/inherit.
>>
>> and in the light of this discussion, 5f83dd03a2 seems incorrect, no?
>
> I agree that commit 5f83dd03a2 is incorrect.  'kodi/wayland' is quite
> clearly a case where 'package/inherit' should be used.

Would you be able to write a bit of doc explaining when both should be
used?  It's a common pitfalls among contributors, and I suspect few of
us have an understanding good enough to write it down in a manner clear
enough to be understood by new contributors.

-- 
Thanks,
Maxim

Re: The package/inherit trap

[Maxim Cournoyer]

Hi,

Attila Lendvai <attila@lendvai.name> writes:

>> How about 'package/variant' or 'package/variant-of'?
>
> +1 for trying to capture the intention in the name, instead of the means of implementation.

I like 'package/variant'.  It should be possible to automatically update
our code base to use it, and deprecate the legacy name.

-- 
Thanks,
Maxim

Re: The package/inherit trap

[Mark H Weaver]

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Mark H Weaver <mhw@netris.org> writes:
>
>> Simon Tournier <zimon.toutoune@gmail.com> writes:
>>> and in the light of this discussion, 5f83dd03a2 seems incorrect, no?
>>
>> I agree that commit 5f83dd03a2 is incorrect.  'kodi/wayland' is quite
>> clearly a case where 'package/inherit' should be used.
>
> Would you be able to write a bit of doc explaining when both should be
> used?  It's a common pitfalls among contributors, and I suspect few of
> us have an understanding good enough to write it down in a manner clear
> enough to be understood by new contributors.

I don't have time to write the doc, but I can offer a few words here.

The fundamental question that needs to be asked, to decide whether to
write (define DERIVED-PACKAGE (package/inherit BASE OVERRIDES ...)) or
(define DERIVED-PACKAGE (package (inherit BASE) OVERRIDES ...)) is this:
if BASE had a replacement, would it make sense to apply the same
OVERRIDES to BASE's replacement to automatically create a replacement
for DERIVED-PACKAGE?

Consider 'kodi/wayland' as an example:

__ (define-public kodi/wayland
____ (package
______ (inherit kodi)
______ (name "kodi-wayland")
______ (arguments
_______ (substitute-keyword-arguments (package-arguments kodi)
_________ ((#:configure-flags flags)
__________ `(cons "-DCORE_PLATFORM_NAME=wayland"
_________________ (delete "-DCORE_PLATFORM_NAME=x11" ,flags)))))
______ (inputs
_______ (modify-inputs (package-inputs kodi)
_________ (prepend libinput
__________________ libxkbcommon
__________________ waylandpp
__________________ wayland-protocols)))
______ (synopsis "Kodi with Wayland rendering backend")))

Suppose, at some future time, 'kodi' were given a replacement named
'kodi-with-fixes' to apply a security update.

The question to ask yourself is this: would it make sense for
'kodi/wayland' to automatically be given a replacement field defined
exactly as above except with (inherit kodi) changed to (inherit
kodi-with-fixes)?

If the answer is "yes", then use (package/inherit BASE OVERRIDES ...),
and make sure that BASE is simply a variable name.

If the answer is "no", use (package (inherit BASE) OVERRIDES ...), in
which case any replacement of BASE will simply be dropped, and the new
package will not have a replacement unless one is explicitly given in
OVERRIDES.

The rationale for 'package/inherit' is to ensure that, e.g., security
updates applied to 'kodi' will automatically be propagated to
'kodi/wayland', and similarly for other packages.

Unless I'm mistaken, the criterion given above is fully general, and
thus sufficient on its own.  However, for the sake of reducing the
cognitive load on contributors, one could proceed to enumerate simpler
heuristics that can be used in common cases.

For example, if OVERRIDES includes a new 'source' field, the next
question to ask is: does the new 'source' field make an incremental
change to (package-source BASE) e.g. by simply adding another patch?  If
so, then 'package/inherit' might do the right thing.  However, if the
new 'source' field doesn't even look at (package-source BASE), then it's
certainly not going to work sensibly when applied to BASE's replacement.

Another common case is when the package you're defining *is* BASE's
replacement.  In that case, you certainly don't want to use
'package/inherit'.

If you want to rename 'package/inherit', I'd suggest something like
'package/inherit-with-replacements'.

Hope this helps.  Feel free to do as you wish without consulting me
further.  I'm using a private branch that hasn't been merged with
upstream Guix since April 2021, so your decisions won't affect me
in any case.

      Mark

Re: The package/inherit trap

[Mark H Weaver]

Hello again,

I made a mistake in my previous message.

Earlier, I wrote:
> Consider 'kodi/wayland' as an example:
>
> __ (define-public kodi/wayland
> ____ (package
> ______ (inherit kodi)
> ______ (name "kodi-wayland")
> ______ (arguments
> _______ (substitute-keyword-arguments (package-arguments kodi)
> _________ ((#:configure-flags flags)
> __________ `(cons "-DCORE_PLATFORM_NAME=wayland"
> _________________ (delete "-DCORE_PLATFORM_NAME=x11" ,flags)))))
> ______ (inputs
> _______ (modify-inputs (package-inputs kodi)
> _________ (prepend libinput
> __________________ libxkbcommon
> __________________ waylandpp
> __________________ wayland-protocols)))
> ______ (synopsis "Kodi with Wayland rendering backend")))
>
> Suppose, at some future time, 'kodi' were given a replacement named
> 'kodi-with-fixes' to apply a security update.
>
> The question to ask yourself is this: would it make sense for
> 'kodi/wayland' to automatically be given a replacement field defined
> exactly as above except with (inherit kodi) changed to (inherit
> kodi-with-fixes)?

The above paragraph isn't quite right.  It should be:

> The question to ask yourself is this: would it make sense for
> 'kodi/wayland' to automatically be given a replacement field defined
> exactly as above except with all free references to 'kodi' within the
> 'package' (or 'package/inherit') form changed to 'kodi-with-fixes'?

Alternatively, for the sake of those who don't know what it means for a
variable reference to be "free" within a given expression, here's
another way to write it:

> The question to ask yourself is this: would it make sense for
> 'kodi/wayland' to automatically be given a replacement field defined
> exactly as above except with the 'package' (or 'package/inherit') form
> wrapped within (let ((kodi kodi-with-fixes)) ...).

This point is crucial.  Observe that in the 'kodi/wayland' definition,
the OVERRIDES for the 'arguments' and 'inputs' fields are defined as
incremental changes to (package-arguments kodi) and (package-inputs
kodi), respectively.

In order for (package/inherit BASE OVERRIDES ...) to work as intended,
the incremental changes made in OVERRIDES should be based upon the
variable BASE, because that is the only variable that will be rebound to
(package-replacement BASE) when automatically producing the replacement
for the derived package.

      Regards,
        Mark

Re: The package/inherit trap

[Maxim Cournoyer]

Hi Mark,

Mark H Weaver <mhw@netris.org> writes:

[...]

> I don't have time to write the doc, but I can offer a few words here.
>
> The fundamental question that needs to be asked, to decide whether to
> write (define DERIVED-PACKAGE (package/inherit BASE OVERRIDES ...)) or
> (define DERIVED-PACKAGE (package (inherit BASE) OVERRIDES ...)) is this:
> if BASE had a replacement, would it make sense to apply the same
> OVERRIDES to BASE's replacement to automatically create a replacement
> for DERIVED-PACKAGE?
>
> Consider 'kodi/wayland' as an example:
>
> __ (define-public kodi/wayland
> ____ (package
> ______ (inherit kodi)
> ______ (name "kodi-wayland")
> ______ (arguments
> _______ (substitute-keyword-arguments (package-arguments kodi)
> _________ ((#:configure-flags flags)
> __________ `(cons "-DCORE_PLATFORM_NAME=wayland"
> _________________ (delete "-DCORE_PLATFORM_NAME=x11" ,flags)))))
> ______ (inputs
> _______ (modify-inputs (package-inputs kodi)
> _________ (prepend libinput
> __________________ libxkbcommon
> __________________ waylandpp
> __________________ wayland-protocols)))
> ______ (synopsis "Kodi with Wayland rendering backend")))
>
> Suppose, at some future time, 'kodi' were given a replacement named
> 'kodi-with-fixes' to apply a security update.
>
> The question to ask yourself is this: would it make sense for
> 'kodi/wayland' to automatically be given a replacement field defined
> exactly as above except with (inherit kodi) changed to (inherit
> kodi-with-fixes)?
>
> If the answer is "yes", then use (package/inherit BASE OVERRIDES ...),
> and make sure that BASE is simply a variable name.
>
> If the answer is "no", use (package (inherit BASE) OVERRIDES ...), in
> which case any replacement of BASE will simply be dropped, and the new
> package will not have a replacement unless one is explicitly given in
> OVERRIDES.
>
> The rationale for 'package/inherit' is to ensure that, e.g., security
> updates applied to 'kodi' will automatically be propagated to
> 'kodi/wayland', and similarly for other packages.
>
> Unless I'm mistaken, the criterion given above is fully general, and
> thus sufficient on its own.  However, for the sake of reducing the
> cognitive load on contributors, one could proceed to enumerate simpler
> heuristics that can be used in common cases.
>
> For example, if OVERRIDES includes a new 'source' field, the next
> question to ask is: does the new 'source' field make an incremental
> change to (package-source BASE) e.g. by simply adding another patch?  If
> so, then 'package/inherit' might do the right thing.  However, if the
> new 'source' field doesn't even look at (package-source BASE), then it's
> certainly not going to work sensibly when applied to BASE's replacement.
>
> Another common case is when the package you're defining *is* BASE's
> replacement.  In that case, you certainly don't want to use
> 'package/inherit'.
>
> If you want to rename 'package/inherit', I'd suggest something like
> 'package/inherit-with-replacements'.
>
> Hope this helps.  Feel free to do as you wish without consulting me
> further.  I'm using a private branch that hasn't been merged with
> upstream Guix since April 2021, so your decisions won't affect me
> in any case.

Thank you, that is abundantly detailed and clear; I'll try adapting it
in our manual.

Cheers!

-- 
Maxim