Subject: Re: [bug#40322] [PATCH v3] gnu: Add Xplanet.

From: R Veera Kumar <[email-masked]>
Date: Wed, 1 Apr 2020 22:38:12 +0530

On Tue, Mar 31, 2020 at 09:50:27AM -0500, Eric Bavier wrote:
> On 31.03.2020 02:57, R Veera Kumar wrote:
> > +       (patches
> > +        (search-patches
> > +         "xplanet-1.3.1-remove-null-comparison.patch"
> > +         "xplanet-1.3.1-libdisplay_DisplayOutput.cpp.patch"
> > +         "xplanet-1.3.1-libimage_gif.c.patch"
> > +         "xplanet-1.3.1-readConfig-fixclang.cpp.patch"
> 
> Is this patch necessary if we're building with gcc?
>

No. It is to build with clang. I thought since Guix also has clang, it
might be useful in future. I will drop it.
 
> > +         "xplanet-1.3.1-xpUtil-Add2017LeapSecond.cpp.patch"))))
> > +    (build-system gnu-build-system)
> > +    (native-inputs
> > +     `(("pkg-config" ,pkg-config)))
> > +    (inputs
> > +       ("giflib" ,giflib)
> > +       ("libjpeg", libjpeg)
> > +       ("libpng" ,libpng)
> > +       ("libtiff" ,libtiff)
> > +       ("zlib" ,zlib)))
> > +    (arguments
> > +     `(#:configure-flags
> > +       (list
> > +        "--without-pnm"       ;; no proper pnm library in guix
> 
> The "netpbm" package provides a pnm library.
> 

I know it. It does not compiles with the current netpbm.
The libnetpbm.so file is not installed by the pkg and xplanet
configure script marks it as not available.

I have filed bug report for this: #40376

It can be packaged without netpbm for now and when the fix is there
I will enable it.

> > diff --git
> > a/gnu/packages/patches/xplanet-1.3.1-remove-null-comparison.patch
> > b/gnu/packages/patches/xplanet-1.3.1-remove-null-comparison.patch
> > new file mode 100644
> > index 0000000000..002701ed0c
> > --- /dev/null
> > +++ b/gnu/packages/patches/xplanet-1.3.1-remove-null-comparison.patch
> > @@ -0,0 +1,161 @@
> > +Origin: Gentoo Harri Nieminen 2017-02-28
> > +Url:
> > https://gitweb.gentoo.org/repo/gentoo.git/log/x11-misc/xplanet/files/
> > ++xplanet-1.3.1-remove-null-comparison.patch
> > +
> > +Index: src/libannotate/addArcs.cpp
> > +===================================================================
> > +diff --git a/src/libannotate/addArcs.cpp b/src/libannotate/addArcs.cpp
> > +--- a/src/libannotate/addArcs.cpp	(revision 206)
> > ++++ b/src/libannotate/addArcs.cpp	(revision 207)
> > +@@ -258,7 +258,7 @@
> > +         {
> > +             ifstream inFile(arcFile.c_str());
> > +             char *line = new char[MAX_LINE_LENGTH];
> > +-            while (inFile.getline (line, MAX_LINE_LENGTH, '\n') !=
> > NULL)
> > ++            while (inFile.getline (line, MAX_LINE_LENGTH, '\n'))
> 
> I'm worried this might not be correct.  The "getline" function always
> returns it's first parameter, which, I think, can never evaluate to a falsy
> value.  The safer option would be to call ".eof()" on the result, and would
> match the intentions.  See e.g. https://notabug.org/bavier/guix-bavier/src/master/bavier/patches/xplanet-cxx11-eof.patch
> 

Okay. I have tested with your patch and builds and works fine.

R Veera Kumar

> -- 
> `~Eric