[bug#63435] Add vcmi 1.2.1 to games.scm

On 2023-05-11, Vagrant Cascadian wrote:
> On 2023-05-11, Liliana Marie Prikler wrote:
>> Am Mittwoch, dem 10.05.2023 um 22:43 -0700 schrieb Vagrant Cascadian:
>>> We probably want to try to apply Debian's "privacy-breach" patch to
>>> disable phoning home, as well as the reproducible builds patch and
>>> some additional reproducibility patches I am working on, but figured
>>> I would submit the package to guix for review now, in case anyone
>>> else wants to join me down this rabbit hole...
>> Well, it won't be added to Guix without such patches, so someone needs
>> to go down this rabbit hole.

Applied the privacy-breach patch, the maxu32/minizip patch was no longer
relevent, and the reproducible builds patches only affect documentation,
which is not currently provided in this package (there are no upstream
rules to build vcmimanual.tex into any documentation).


>>> +     "@code{vcmi} is an implementation of Heroes of Might and
>>> +Magic III (aka HOMM3) game engine.  It requires assets and game
>>> resources to
>>> +play; it will look for them at @file{~/.local/share/vcmi} folder.")
>> Use an @acronym or just spell it out.
>
> Presuming you mean "(aka HOMM3)" ... honestly, maybe best to just drop
> it entirely; I am not sure it adds much.

Dropped it.


V2 patch attached.


Thanks for reviewing!


live well,
  vagrant

Am Donnerstag, dem 11.05.2023 um 16:07 -0700 schrieb Vagrant Cascadian:

> +(define-public vcmi
> +  (package
> +    (name "vcmi")
> +    (version "1.2.1")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/vcmi/vcmi")
> +                    (commit version)
> +                    (recursive? #t)))
Can we do without the recursive checkout?
> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +               
> "1nx3i078cxkak2ci514pf4pgi5269mp08njynsg35pin4yp3fn0p"))
> +              (patches (search-patches "vcmi-disable-privacy-
> breach.patch"))))
IIRC the reproducible builds patch is still missing, right?

> +    (native-inputs (list boost
Guix style is, like, a suggestion that can be wrong.  You are allowed
to fight it when the result of doing so is demonstrably better.

Cheers

On 2023-05-12, Liliana Marie Prikler wrote:
> Am Donnerstag, dem 11.05.2023 um 16:07 -0700 schrieb Vagrant Cascadian:
>
>> +(define-public vcmi
>> +  (package
>> +    (name "vcmi")
>> +    (version "1.2.1")
>> +    (source (origin
>> +              (method git-fetch)
>> +              (uri (git-reference
>> +                    (url "https://github.com/vcmi/vcmi")
>> +                    (commit version)
>> +                    (recursive? #t)))
> Can we do without the recursive checkout?

There is one component still used with the recursive
checkout. ... AI/Fuzzy* I think? I do not know if it could be built
independently, but I have not seriously looked into it.

If tests were enabled, the googletest stuff might be needed; it was a
bit unclear to me if the googletest packaged in guix could
work. Regardless, tests are disabled upstream... so if there is a way to
only download one and not the other, I guess that would save some
bandwith.

I *think* those are the only two things pulled in.


>> +              (file-name (git-file-name name version))
>> +              (sha256
>> +               (base32
>> +               
>> "1nx3i078cxkak2ci514pf4pgi5269mp08njynsg35pin4yp3fn0p"))
>> +              (patches (search-patches "vcmi-disable-privacy-
>> breach.patch"))))
> IIRC the reproducible builds patch is still missing, right?

The Debian package implements building man pages and documentation
outside of the upstream build system...

It did not seem worth patching something that was not used to build
anything... the reproducible builds patch(es) only apply to
documentation which is not part of the upstream build process, so I left
it out of this iteration.

That said...

Building vcmimanual.tex appears to be a one-liner, pulling in some tex
related dependencies:

  https://salsa.debian.org/games-team/vcmi/-/blob/master/debian/rules#L56

And generating manpages used help2man and some templates debian ships:

  https://salsa.debian.org/games-team/vcmi/-/blob/master/debian/rules#L46-48

Not sure if the manpages are worth the effort, or if the manual is worth
the larger dependency tree...


>> +    (native-inputs (list boost
> Guix style is, like, a suggestion that can be wrong.  You are allowed
> to fight it when the result of doing so is demonstrably better.

I get that ... but I also like just being able to run guix style and not
having to make those judgement calls. Because other things guix style
may change that are a good idea and it is really difficult to pick and
choose which things to revert and which to keep over time...

There are some things I think guix style does wrong(in particular, I
always prefer one input per line to make diffs easier to read), but I do
not hold strong opinions on guile coding style and just prefer to
concede to guix style and bear with the results.

I am also not strongly opinionated (it goes both ways, I guess!)... so
for clarity, are you saying you would prefer:

  (native-inputs
    (list
      boost
      ...
or:

  (native-inputs
    (list boost
          ...

or something else?


live well,
  vagrant

Am Donnerstag, dem 11.05.2023 um 23:17 -0700 schrieb Vagrant Cascadian:
> On 2023-05-12, Liliana Marie Prikler wrote:
> > Am Donnerstag, dem 11.05.2023 um 16:07 -0700 schrieb Vagrant
> > Cascadian:
> > 
> > > +(define-public vcmi
> > > +  (package
> > > +    (name "vcmi")
> > > +    (version "1.2.1")
> > > +    (source (origin
> > > +              (method git-fetch)
> > > +              (uri (git-reference
> > > +                    (url "https://github.com/vcmi/vcmi")
> > > +                    (commit version)
> > > +                    (recursive? #t)))
> > Can we do without the recursive checkout?
> 
> There is one component still used with the recursive
> checkout. ... AI/Fuzzy* I think? I do not know if it could be built
> independently, but I have not seriously looked into it.
fuzzylite can be taken from the system which is the preferred approach.

> 
> If tests were enabled, the googletest stuff might be needed; it was a
> bit unclear to me if the googletest packaged in guix could
> work. Regardless, tests are disabled upstream... so if there is a way
> to only download one and not the other, I guess that would save some
> bandwith.
> 
> I *think* those are the only two things pulled in.
Not that it matters if we aren't building tests, but googletest can and
should be unbundled.  There's a fair number of packages already setting
a precedent.

> > > +              (file-name (git-file-name name version))
> > > +              (sha256
> > > +               (base32
> > > +               
> > > "1nx3i078cxkak2ci514pf4pgi5269mp08njynsg35pin4yp3fn0p"))
> > > +              (patches (search-patches "vcmi-disable-privacy-
> > > breach.patch"))))
> > IIRC the reproducible builds patch is still missing, right?
> 
> The Debian package implements building man pages and documentation
> outside of the upstream build system...
> 
> It did not seem worth patching something that was not used to build
> anything... the reproducible builds patch(es) only apply to
> documentation which is not part of the upstream build process, so I
> left it out of this iteration.
> 
> That said...
> 
> Building vcmimanual.tex appears to be a one-liner, pulling in some
> tex related dependencies:
> 
>  
> https://salsa.debian.org/games-team/vcmi/-/blob/master/debian/rules#L56
> 
> And generating manpages used help2man and some templates debian
> ships:
> 
>  
> https://salsa.debian.org/games-team/vcmi/-/blob/master/debian/rules#L46-48
> 
> Not sure if the manpages are worth the effort, or if the manual is
> worth the larger dependency tree...
Fair enough, if it can be left without, let's do without (unless you
really want to build the manpage).  Alternatively, you can pull the
inputs in, but phrase the (build-documentation ...) phase in a way that
those inputs can be dropped if someone values their disk space.

> > > +    (native-inputs (list boost
> > Guix style is, like, a suggestion that can be wrong.  You are
> > allowed
> > to fight it when the result of doing so is demonstrably better.
> 
> I get that ... but I also like just being able to run guix style and
> not having to make those judgement calls. Because other things guix
> style may change that are a good idea and it is really difficult to
> pick and choose which things to revert and which to keep over time...
> 
> There are some things I think guix style does wrong(in particular, I
> always prefer one input per line to make diffs easier to read), but I
> do not hold strong opinions on guile coding style and just prefer to
> concede to guix style and bear with the results.
> 
> I am also not strongly opinionated (it goes both ways, I guess!)...
> so for clarity, are you saying you would prefer:
> 
>   (native-inputs
>     (list
>       boost
>       ...
> or:
> 
>   (native-inputs
>     (list boost
>           ...
> 
> or something else?
The latter.  If it ever comes to needing (list on its own line you
better have a good explanation for that or fix your comments so that
they don't go overboard.


Cheers