#20692: Add sage-apply-patches helper script for use in spkg-install scripts
-------------------------------------+-------------------------------------
Reporter: embray | Owner:
Type: enhancement | Status: needs_work
Priority: minor | Milestone: sage-7.3
Component: build | Resolution:
Keywords: | Merged in:
Authors: Erik Bray | Reviewers: Jeroen Demeyer
Report Upstream: N/A | Work issues:
Branch: u/embray/patch- | Commit:
cleanup | 29bd19903b16289c178e9468194318e5d1017dad
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by embray):
Replying to [comment:7 jdemeyer]:
> Following up on [comment:2]: it's very useful to support an empty set of
patches. For example, when upgrading a package, it can happen that all
patches need to be removed. You don't want that to break the build. That
is also the reason why many packages have code to apply patches when they
don't have patches. I would not remove such code since it allows easily
adding a new patch. Ideally, every `spkg-install` script would call `spkg-
apply-patches`.
I guess I was thinking it was wasteful to call a process (especially on
Windows where process creation is much slower) when it's not needed.
Granted, when this was inline in every script it didn't require a process
creation (more on that in a followup). I think if someone wants to add a
patch to a package they should explicitly add `spkg-appy-patches` as well.
If they forget to do that, well, then they clearly didn't test that their
patch applies at build time :)
As for it being an error when no patches are found, I have no strong
feelings. I liked that it was explicit, but it could just as will either
print a message, or go completely silent.
> A minor thing: I would prefer to remove the -pNUM option. It's better if
we consistently use -p1 in all cases. That's also how git generates its
patches.
That's fine by me. I think there was one case, maybe two where it was
needed, but it would be just as easy to update those patch files and keep
things consistent throughout.
> Why not always use the option `--no-backup-if-mismatch` instead of only
for PARI?
I figured maybe there was a reason to prefer to keep backups in general.
Would be fine with me to apply in general though. Previously there was
one other case where additional arguments had to be passed to `patch`, but
then I realized that package had no patches anymore.
> I don't get this:
and it is assumed that the patches are being applied from
the root of the package source
Meaning the patches are applied after `cd`-ing into the upstream source,
hence the default path to search for patches being `../patches`. No
reason it has to be that way but it was the most minimal change from the
existing code.
--
Ticket URL: <http://trac.sagemath.org/ticket/20692#comment:8>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
