Fix hardcoded 'make' in NTL's build scripts and track all files

[leif]

New spkg: ​http://boxen.math.washington.edu/home/vbraun/spkg/ntl-5.5.2.p3.spkg

ntl-5.5.2.p2 (Leif Leonhardy, Jean-Pierre Flori, 6 June 2013)

• #14692: Patch upstream to use $(MAKE) (instead of make) in the generated Makefile (and in two scripts called from the Makefile which in turn invoke make). As a consequence, also some parts are now properly built in parallel if make was told to use multiple jobs.
• Minor clean-up of spkg-install and spkg-check.
• Refactor and rename patches.

ntl-5.5.2.p3 (Volker Braun, July 10 2013)

• #14692: Add spkg-src and track all files.

For review purposes

• p0->p1: ntl-5.5.2.p0-p1.diff​
• p1->p2: ntl-5.5.2.p2.patch​
• p2->p3: ntl-p2-p3.diff​

[leif]

Diff between the .p0 and the .p1. For reference / review only.

[jdemeyer]

I think it would be much better to have just one patch fixing this issue, instead of 3 (one per file). This makes it easier to document what the patch does (SPKG.txt should mention this patch).

Also, having just one patch makes it easier to report upstream (which should be done).

[jpflori]

Same feeling as Jeroen. If you also feel like contacting Shoup to integrate all of our patches, please go ahead :)

[leif]

That's (to some extent) a matter of taste, although I actually prefer [cumulative] patches with "verbose names", too. (It only sometimes gets messy when different patches patch the same files, and especially when they're applied in some "random" order.)

But there was already patches/mfile.patch (fixing Cygwin stuff^*), so I followed the scheme that was already there.

---

^* Does anyone recall OTOH who added that? XD

[jpflori]

I guess I'm the culprit for the Cygwin patch... As all the previous patches only have one purpose and touch only one file, it may be a good time to rename them, DoConfig? and mfile are not really talkative.

Of course patching the same file multiple times with your new set of patches might be problematic.

[jpflori]

I'd say the easiest way to go now is to rebase this ticket on the trivial change of #2114.

[leif]

Replying to jpflori:

I'd say the easiest way to go now is to rebase this ticket on the trivial change of #2114.

Au contraire I'd say (if that's correctly spelled).

[leif]

Is "refactor and rename patches" in any way related to the ticket's subject, namely fixing a defect (which the current spkg properly does)?

Honestly, if you feel all the patches should be restructured (including their documentation), that can be done on another ticket.

[jdemeyer]

Replying to leif:

Is "refactor and rename patches" in any way related to the ticket's subject, namely fixing a defect (which the current spkg properly does)?

Perhaps what is meant is simply my comment about having one patch to fix make and documenting that patch in SPKG.txt.

[jpflori]

Yeah, let's just do that.

To properly refactor we should split the libtool part in DoConfig?.patch and merge it with mfile.patch into a libtool_flag.patch later and put the remaining part into a disable_help.patch. Maybe move new.h.patch to new_singular.patch. And properly document everything in SPKG.txt by reflecting what is done by which patch (although the names should be then more talkative). But I can do that in a folow up ticket.

[jpflori]

I've put an spkg doing all the above and based on #2114 at:

• ​http://boxen.math.washington.edu/home/jpflori/spkg/ntl-5.5.2.p2.spkg

[jpflori]

Spkg diff, for review only.

[leif]

patches/make.patch lacks the patch to src/src/TestScript.

(I'd also rename the patch to use_MAKE.patch or smth like that.)

[leif]

"Patches" is usually a subsection of "Special Update/Build Instructions", i.e., === Patches ===.

[leif]

And if you like, do

patches/make.patch

@@ -31 +31 @@
  
  setup4:
 -       sh Wizard $(WIZARD)
-+       +sh Wizard $(WIZARD) # shell script invoking a Perl script invoking \$MAKE
++       +sh Wizard $(WIZARD) # shell script invoking a Perl script invoking $$MAKE
  
  
  ntl.a: $(OBJ) 
@@ -44 +44 @@
         ./QuickTest
         sh RemoveProg QuickTest
 -       sh TestScript
-+       +sh TestScript # shell script invoking \$MAKE
++       +sh TestScript # shell script invoking $$MAKE
  
  #################################################################
  #

which fixes a minor flaw I only noticed after I had already uploaded my original spkg...

[jdemeyer]

Why did you remove the patches involving LIBTOOL_LINK_FLAGS? This doesn't seem to be documented.

[leif]

Replying to jdemeyer:

Why did you remove the patches involving LIBTOOL_LINK_FLAGS?

? They're (now) in patches/libtool_flag.patch.

This doesn't seem to be documented.

[leif]

Still, we have

 * libtool_flag.patch: modify mfile to enable the build of a shared library
   on Cygwin

but

$ grep '^+++' patches/libtool_flag.patch 
+++ src/src/DoConfig    2012-08-07 12:17:12.828358360 +0200
+++ src/src/mfile       2013-06-05 20:43:25.845240069 +0200

I.e., add DoConfig to the patch's documentation.

[jdemeyer]

Replying to leif:

They're (now) in patches/libtool_flag.patch.

I don't see that file in ntl-5.5.2.p2.patch​. Did you forget to hg add that file?

[jdemeyer]

Making some changes to the spkg, hang on...

[leif]

Replying to jdemeyer:

Replying to leif:

They're (now) in patches/libtool_flag.patch.

I don't see that file in ntl-5.5.2.p2.patch​. Did you forget to hg add that file?

.hgignore is the problem. (It contains libtool rather than libtool/.)

[leif]

Replying to jdemeyer:

Replying to leif:

They're (now) in patches/libtool_flag.patch.

Did you forget to hg add that file?

And yes, fixing .hgignore, indeed one gets:

? patches/libtool_flag.patch

[jdemeyer]

Better wait for a review of #2114 first, otherwise we can be rebasing forever.

[vbraun]

Superseded by #14876

[vbraun]

Since the new upstream version is incompatible with our Singular, lets make a new spkg here with the fixes needed for the git transition. The version bump can wait until Singular is ready.

[vbraun]

For review purposes only

[jpflori]

Is the "Make everything writable" part actually needed? I guess so, or you wouldn't have added it, but I was not aware of that.

[jpflori]

It would be nice to have a way to use a local copy of ntl tarball in spkg-src (just as you did for Singular in #14737).

[vbraun]

I haven't checked if the "make writeable" part is really needed. But since it also depends on umask and since wrong permissions in tarballs are not that uncommon, I think its better to include it.

The ntl tarball is tiny and on a fast server, so I didn't need an option to use a local copy for writing/testing the script. If you need it, feel free to open a ticket with that feature request.

[jpflori]

Replying to vbraun:

I haven't checked if the "make writeable" part is really needed. But since it also depends on umask and since wrong permissions in tarballs are not that uncommon, I think its better to include it.

The ntl tarball is tiny and on a fast server, so I didn't need an option to use a local copy for writing/testing the script. If you need it, feel free to open a ticket with that feature request.

Except when the site is down :)

[jpflori]

Maybe we could remove autom4te.cache from the libtool dir.

[jpflori]

Otherwise everything is fine.

[vbraun]

I've added a rm -rf autom4te.cache to spkg-src and re-ran it.