Vote on making GNU patch a standard package
David Kirkby
to Sage can be made in a more sensible manner than using 'cp' as now.
(There's no point in 'patch' being optional at all, as it would be
needed when building Sage).
For
* It is small - the source code is about 240 KB, so a Sage package
could be a similar size.
* It would be easy to maintain - we will rarely if ever need to update it.
* It will allow small patches to be made to Sage, without the extra
bulk that copying files makes
* It should reduce the chance of one patch screwing up another (see
my post about Singular)
* The amount of code that needs to be added to trac would be
significantly reduced. Currently making one small patch to a large
file in Sage means the Mercurial patch is huge, making it more
difficult to review.
* It would avoid the need to maintain both patched files and diff
files, and keep them in sync. I showed an example yesterday where one
of the patches to Python has a diff file that is older than the
changed source file.
* My removing the need to have large files copied, we could actually
reduce the size of Sage, though this is not going to happen overnight
- nobody is likely to want to elect to remove all patches that use
'cp' and replace them by ones that use 'patch'.
* Since everyone will have the same version of patch, there should be
no issues like different patch commands behaving differently.
* I don't mind creating the package - it would be a simple task.
* I don't mind taking on the role of maintainer for 2 years
Against.
* It adds more to Sage.
* It could not be optional/experimental. One would have to make a
decision for it to be 'standard' from the start. Otherwise it would
form no useful function at al.
* Mercurial can arguably be used, but this is difficult if not
impossible in my opinion. The version of Python shipped with Solaris
is too old to work with Mercurial, so we need a recent python to build
Mercurial. But Python needs patches to be built. We could patch Python
by using 'cp' then switch to Mercurial after it is built, but that
needs two different methods of patching Sage. (I've already shown
there is a bad patch in Python).
So do you vote
[Yes] Include GNU patch as a standard package in Sage
[No] Do not include it.
Dave
François Bissey
Francois
John Cremona
bzip2), unless there are downsides which I have not thought of.
John
> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org
>
Adam Webb
[Yes] Include GNU patch as a standard package in Sage
Adam
Tim Daly
However, given that developers are the only people likely
to know how to create and post a diff-Naur patch file and
developers are likely able to install tools and 'patch' is
a well-known, widely used, and standard tool ...
does it make sense to have your own copy?
Ivan Andrus
-Ivan
David Kirkby
> It is officially none of my business what you decide.
> However, given that developers are the only people likely
> to know how to create and post a diff-Naur patch file and
> developers are likely able to install tools and 'patch' is
> a well-known, widely used, and standard tool ...
>
> does it make sense to have your own copy?
I think you may have missed the point Tim.
The aim is not to expand Sage so it can be a patching tool.
When a non-developer installs Sage, he/she patches a number of files
in Sage, though that is not apparent to him/her. A script will do this
for them. So *anyone* that builds Sage from source will be patching
probably 50 or more files already. Currently that is done by using
'cp', but I'd propose instead we use 'patch'.
Dave
Tim Daly
kcrisman
> > However, given that developers are the only people likely
> > to know how to create and post a diff-Naur patch file and
> > developers are likely able to install tools and 'patch' is
this kind. Dave is right that this is beside the point under
discussion, but Sage has consistently been trying to attract people to
developing/documentation/whatever who wouldn't know a diff-Naur patch
file if it hit them in the head, so this argument doesn't necessarily
hold from that side either. Luckily, Sage via Mercurial has something
equivalent to this which is well documented, just for folks like me;
it's worth making the barrier to entry to development (on the
technical side, not mathematical) as low as is reasonable.
In fact, sometimes we tell people on sage-support to apply a patch to
get some bugfix or new functionality, and it's very nice that we can
give them a specific *Sage* command to do this which doesn't require
'tools'. Anyway, perhaps a little off topic, but worth pointing
out :)
- kcrisman
Jason Grout
> I propose that we make GNU patch a standard package, so that patches
> to Sage can be made in a more sensible manner than using 'cp' as now.
> (There's no point in 'patch' being optional at all, as it would be
> needed when building Sage).
>
For the discussion, I believe David is referring to patches in Sage
spkgs (which currently consist of the patched file, and are "applied"
by copying them over the original source file in the spkg src directory).
+1 for the idea of inclusion. I've wanted this for a long time.
Thanks,
Jason
kcrisman
this, though it would have made my life easier a few times. However,
I do recommend that this should not be included unless it is
immediately used in a few of the worst offending spkgs - otherwise it
would seem sort of pointless. Also, whatever ticket adds this should
(in my opinion only) be required to update the source to
http://www.sagemath.org/doc/developer/producing_spkgs.html in a very
explicit way.
I think drkirkby implicitly has said he will be doing these things in
his updates for OpenSolaris etc., so probably it's a moot point, but
no one had said this yet (in this thread).
- kcrisman
William Stein
I vote NO to including patch in sage.
1. We can accomplish the same thing when creating the spkg. E.g, the command
sage -pkg foo-1.2.3
could *automatically* apply patch using each file in
foo-1.2.3/patches/ and create the corresponding patched files. To
*creators* of spkg's this is functionally equivalent to what is
proposed, and perhaps even better, because use of patch files will be
carefully organized, instead of being done by adhoc use of the patch
command in spkg-install. Also, no current spkg-install's have to be
changed at all. By apply patch at runtime, we only assume the user
has the ability to patch files, and since Mercurial is in Sage this
adds no dependencies.
2. We should never, ever add any new packages to sage without being
very, very careful first. Every package added to sage increases later
porting work, maintenance work, etc. forever. This is particularly a
concern to *me*, since some of you come and go, but I'll be dealing
with sage pretty much forever.
-- William
David Kirkby
> I vote NO to including patch in sage.
>
> 1. We can accomplish the same thing when creating the spkg. E.g, the command
>
> sage -pkg foo-1.2.3
>
> could *automatically* apply patch using each file in
> foo-1.2.3/patches/ and create the corresponding patched files. To
> *creators* of spkg's this is functionally equivalent to what is
> proposed, and perhaps even better, because use of patch files will be
> carefully organized, instead of being done by adhoc use of the patch
> command in spkg-install.
I was not suggesting it to be used by the creators of packages - they
need a 'diff' command, not a patch command.
> Also, no current spkg-install's have to be
> changed at all.
No, but you know as well as I do, that this is an ongoing process. I
don't know how many files in Sage get patched, but it is a lot of
them.
> By apply patch at runtime, we only assume the user
> has the ability to patch files, and since Mercurial is in Sage this
> adds no dependencies.
In order to have Mercurial you need Python, which is currently patched
multiple times. Python is certainly one of the packages that has a
great many patches. The Python that comes with Solaris is too old to
build Mercurial to to use Mercurial one would need to
> 2. We should never, ever add any new packages to sage without being
> very, very careful first. Every package added to sage increases later
> porting work, maintenance work, etc. forever. This is particularly a
> concern to *me*, since some of you come and go, but I'll be dealing
> with sage pretty much forever.
>
> -- William
I would have expected the maintenance on 'patch' to be very low
indeed. It has no dependencies other than a C compiler and 'make'.
There are no optional things you can enable with options to the
configure script.
Take a look if you want
http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
I wont create a ticket for it, since you are obviously not keen, but I
thought I'd post a link given I'd made the package anyway.
On my Ultra 27, tt takes a grand total of 3.2 seconds to install it.
real 0m3.191s
user 0m2.698s
sys 0m1.640s
Successfully installed patch-2.6.1
though that increases to 3.6 seconds if I run the self-tests.
24 tests (24 passed, 0 failed)
All tests succeeded!
Now cleaning up tmp files.
rm: Cannot remove any directory in the path of the current working directory
/export/home/drkirkby/sage-4.5.alpha1/spkg/build/patch-2.6.1
Making Sage/Python scripts relocatable...
Making script relocatable
Finished installing patch-2.6.1.spkg
real 0m3.635s
user 0m3.057s
sys 0m2.392s
Dave
William Stein
> On 1 July 2010 18:02, William Stein <wst...@gmail.com> wrote:
>
>> I vote NO to including patch in sage.
>>
>> 1. We can accomplish the same thing when creating the spkg. E.g, the command
>>
>> sage -pkg foo-1.2.3
>>
>> could *automatically* apply patch using each file in
>> foo-1.2.3/patches/ and create the corresponding patched files. To
>> *creators* of spkg's this is functionally equivalent to what is
>> proposed, and perhaps even better, because use of patch files will be
>> carefully organized, instead of being done by adhoc use of the patch
>> command in spkg-install.
>
> I was not suggesting it to be used by the creators of packages - they
> need a 'diff' command, not a patch command.
Patch would be used explicitly by developers in writing the spkg-installs.
>
>> Also, no current spkg-install's have to be
>> changed at all.
>
> No, but you know as well as I do, that this is an ongoing process. I
> don't know how many files in Sage get patched, but it is a lot of
> them.
My suggestion requires changing no spkg-install files; your involves
changing all of them.
Mine does involve rewriting patches/ directories though.
>> By apply patch at runtime, we only assume the user
>> has the ability to patch files, and since Mercurial is in Sage this
>> adds no dependencies.
>
> In order to have Mercurial you need Python, which is currently patched
> multiple times. Python is certainly one of the packages that has a
> great many patches. The Python that comes with Solaris is too old to
> build Mercurial to to use Mercurial one would need to
In my proposal patch is *only* used when typing
sage -pkg foo-1.2.3
At that point, one likely has Mercurial/Python, etc., installed and working.
> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org
>
--
William Stein
Professor of Mathematics
University of Washington
http://wstein.org
David Kirkby
> My suggestion requires changing no spkg-install files; your involves
> changing all of them.
> Mine does involve rewriting patches/ directories though.
> William Stein
As I made clear, I doubt anyone would go around changing the patches
which are currently patched with 'cp'. Only when they need to be
patched again would anyone change the method of applying a patch.
Gradually over time, the number of patches applied with 'cp' would
decrease and the number with 'patch' increase.
If you look at this thread, and the other on Singular, you will find a
large number of people believe adding 'patch' is a good idea, these
include, but are probably not limited to:
Myself
François Bissey
John Cremona
Adam Webb
Ivan Andrus
Jason Grout
Others have expressed dismay at the way the patching currently works.
Anyway, its your project. I can live with using 'cp' - I just don't
think it is a very good idea.
Dave
William Stein
> On 1 July 2010 19:26, William Stein <wst...@gmail.com> wrote:
>
>> My suggestion requires changing no spkg-install files; your involves
>> changing all of them.
>> Mine does involve rewriting patches/ directories though.
>
>> William Stein
>
> As I made clear, I doubt anyone would go around changing the patches
> which are currently patched with 'cp'. Only when they need to be
> patched again would anyone change the method of applying a patch.
> Gradually over time, the number of patches applied with 'cp' would
> decrease and the number with 'patch' increase.
>
> If you look at this thread, and the other on Singular, you will find a
> large number of people believe adding 'patch' is a good idea, these
> include, but are probably not limited to:
>
> Myself
> François Bissey
> John Cremona
> Adam Webb
> Ivan Andrus
> Jason Grout
>
> Others have expressed dismay at the way the patching currently works.
My proposal does involve using patch. It's just that patch is used
automatically by
"sage -pkg" rather than explicitly in spkg-install.
-- William
>
> Anyway, its your project. I can live with using 'cp' - I just don't
> think it is a very good idea.
>
> Dave
>
David Kirkby
> On Thu, Jul 1, 2010 at 11:47 AM, David Kirkby <david....@onetel.net> wrote:
>> Others have expressed dismay at the way the patching currently works.
>
> My proposal does involve using patch. It's just that patch is used
> automatically by
> "sage -pkg" rather than explicitly in spkg-install.
>
> -- William
I don't understand your proposal. Would it need the patch command
added to Sage? I don't understand your method, so can't comment
really.
PS,
Any chance of you reviewing
http://trac.sagemath.org/sage_trac/ticket/9399
You might know why that sun workaround was added in the first place.
Perhaps it was to perhaps support Solaris 9, which we are not
targeting - Solaris 10 came out in 2005. The header file that did not
get included on Solaris exists in the very first release of Solaris
10, so I don't see the point in doing anything different on Solaris to
any other operating system.
Dave
Mike Hansen
> I don't understand your proposal. Would it need the patch command
> added to Sage? I don't understand your method, so can't comment
> really.
William's proposal is to
1) Standardize the filenames of patches so that the only file which
patches ./src/foo/bar/xyz.py is ./patches/foo/bar/xyz.py.patch
2) Only the .patch file is checked into the repository.
3) When doing "sage -spkg" to create an spkg, it goes through all of
the patches under ./patches/ and uses the patch command to make the
patched file which is copied over. In the above example, the "sage
-spkg" script would automatically make ./patches/foo/bar/xyz.py from
./src/foo/bar/xyz.py and ./patches/foo/bar/xyz.py.patch. Thus, only
patch needs to be installed on the machine which creates the original
spkg.
One issue that I have with the way things are done now which is not
addressed by William's proposal is that (logical) patches which touch
multiple files have to be broken up. Similarly, if two (logical
patches) touch the same file, then they have to be merged. If you
used the patch command, then things wouldn't have to be broken up by
file.
--Mike
William Stein
> On Thu, Jul 1, 2010 at 12:05 PM, David Kirkby <david....@onetel.net> wrote:
>> I don't understand your proposal. Would it need the patch command
>> added to Sage? I don't understand your method, so can't comment
>> really.
>
> William's proposal is to
>
> 1) Standardize the filenames of patches so that the only file which
> patches ./src/foo/bar/xyz.py is ./patches/foo/bar/xyz.py.patch
>
> 2) Only the .patch file is checked into the repository.
>
> 3) When doing "sage -spkg" to create an spkg, it goes through all of
> the patches under ./patches/ and uses the patch command to make the
> patched file which is copied over. In the above example, the "sage
> -spkg" script would automatically make ./patches/foo/bar/xyz.py from
> ./src/foo/bar/xyz.py and ./patches/foo/bar/xyz.py.patch. Thus, only
> patch needs to be installed on the machine which creates the original
> spkg.
>
> One issue that I have with the way things are done now which is not
> addressed by William's proposal is that (logical) patches which touch
> multiple files have to be broken up.
A slight modification of what you describe above addresses this.
Just replace: "When doing "sage -spkg" to create an spkg, it goes through all of
the patches under ./patches/ and uses the patch command to make the
patched file which is copied over." by
"When doing "sage -spkg" to create an spkg, it goes through all of
the patches under ./patches/ and uses the patch command to make the
patched FILES which are copied over."
There is no reason that the patches under patches/ have to apply to
only one file.
-- William
> Similarly, if two (logical
> patches) touch the same file, then they have to be merged. If you
> used the patch command, then things wouldn't have to be broken up by
> file.
>
> --Mike
>
Nils Bruin
affects the procedure for making spkgs. What I have done the last
couple of times that I made an spkg update is:
1. sage -sh
2. tar xjf package.p0.spkg
3. replace src with the new version
4. (re)place files in package.p0/patches
5. edit spkg-install
6. cp -r package.p0 package.buildtry
7. run package.buildtry/spkg-install
8. test that this spkg worked out OK
9. tar cjf package.p1.spkg package/
I guess this procedure needs adjustment from step 6 onwards in
William's proposal.
Is "sage -spkg" going to meddle with spkg-install? Can we still
recover a "source" directory from the sage -spkg produced package on
which we can run sage -spkg again to obtain the same package?
In more mathematical terms, "tar cjf" and "tar xjf" are functionally
two-sided inverses of each other, which makes it very transparent how
one can manipulate spkgs. Does "sage -spkg" have a similar "almost
inverse"? It seems to me that William's suggestion would mean that
"tar xjf" definitely wouldn't do anymore.
Incidentally, it seems to me that after running spkg-install, one
cannot make a correct spkg from that directory anymore, because src/
is now patched. That does slow down the development cycle for spkgs a
bit. Do people have smart workarounds for that?
William Stein
> I am not completely sure that I understand how William's proposal
> affects the procedure for making spkgs. What I have done the last
> couple of times that I made an spkg update is:
> 1. sage -sh
> 2. tar xjf package.p0.spkg
> 3. replace src with the new version
> 4. (re)place files in package.p0/patches
> 5. edit spkg-install
> 6. cp -r package.p0 package.buildtry
> 7. run package.buildtry/spkg-install
> 8. test that this spkg worked out OK
> 9. tar cjf package.p1.spkg package/
> I guess this procedure needs adjustment from step 6 onwards in
> William's proposal.
NO. In fact, what I'm proposing wouldn't make any sense to you,
since you seem to not know some basic things about how to make spkg's.
The right way to make and test an spkg is to do:
sage -pkg package.p0
sage -f -m package.p0.spkg
You should never have to explicitly run spkg-install, or use the tar command.
In particular, 9 above is very wrong: you should do "sage -pkg
package.p0" to make
the package. This will run consistency checks on package.p0.
-- William
> Is "sage -spkg" going to meddle with spkg-install?
There is no "sage -spkg".
The command "sage -pkg" won't meddle with spkg-install.
My entire proposal is:
Modify "sage -pkg" so that it generates the patched files from the patches.
That's it.
> Can we still
> recover a "source" directory from the sage -spkg produced package on
> which we can run sage -spkg again to obtain the same package?
Yes.
> In more mathematical terms, "tar cjf" and "tar xjf" are functionally
> two-sided inverses of each other, which makes it very transparent how
> one can manipulate spkgs. Does "sage -spkg" have a similar "almost
> inverse"? It seems to me that William's suggestion would mean that
> "tar xjf" definitely wouldn't do anymore.
>
NO. "tar xjf" will continue to work exactly as before.
> Incidentally, it seems to me that after running spkg-install, one
> cannot make a correct spkg from that directory anymore, because src/
> is now patched. That does slow down the development cycle for spkgs a
> bit. Do people have smart workarounds for that?
You should never, ever, ever run ./spkg-install directly. See above.
And in case people missed it, my entire proposal is:
Modify "sage -pkg" so that it generates the patched files from the patches.
There are details to how that is done, which I haven't specified.
-- William
Georg S. Weber
to say that we should be careful about whether additional spkgs are
really needed.
I vote "-1" to the inclusion of an additional "patch.spkg".
May I assume there is consensus about mercurial (once successfully
installed) provides all the functionality that an additional
"patch.spkg" would provide (maybe even make vanish WIndows/Unix
differences in line ending char conventions in "diff" files, which
probably is an issue for a "plain patch")? If so, reading through the
arguments above, such an additional "patch.spkg" would only be really
needed if both of the following holds:
a) patches have to be applied to a spkg that is a build dependency of
mercurial
b) these patches have to be created from diffs during the build, i.e.
are not already contained in this spkg
Currently, such patches are shipped with spkgs, so this situation does
not occur because "b" is not fulfilled.
And even if at some point in the future, we opted for reducing the
size of sage tarballs by throwing out patches inside spkgs, and
keeping only the diffs, then still those spkgs which are build
dependencies for mercurial could be excluded from that rule (python
itself is one, of course, but hopefully does not need to be patched
forever. OTOH, the vast majority of the 100 or so spkgs in Sage aren't
build dependencies for mercurial).
As William started to point out, the desire to enhance the current way
"/patches" is used, does not necessitate an additional "patch.spkg"
either.
Of course there is room for improvement in the way the "/patches/"
subdirectories of spkgs are handled. If asked for a proposal, I'd
answer that we should have under "/patch/" diff files "20100101.diff",
"20100202.diff", ... that are to be applied in the order given by
their names, creating a certain set of patches (patch files). These
patch files are being stored directly under "/patch/" unless there are
two with exactly the same names, or some files ending in ".diff" (but
only then), in which case these are stored in a subdirectory structure
under "/patch/src/" mimicking "/src/".
Only the diff files need to be in the spkg revision history, the
others are created manually (or by some automatism during spkg
creation --- I would vote against removing the patch files fom the
spkgs, it's just too convenient to have them right there at hand).
All files under "/patch/" except the diff files get copied over during
the build of the spkg, the code doing this could be rather generic
(needs only to check into which subdirectory of "/src/" those "non-
diff"-files lying right under "/patches/" are to be copied to), and
needs not be part of the spkg-install's, but could be called from
these.
But I think we already have too many proposals floating around in this
thread ...
Cheers,
Georg
William Stein
My proposal is very vague and leaves a lot open to interpretation and nailing
down details. Your suggestion above is definitely one possible way to go.
Thanks for making it.
William
Tom Boothby
John Cremona
source files, which everyone seems to agree on. I thought from
earlier postings that this requires having the patch function
installed, so voted in favour. But now it is quite clear that this is
*not* necessary, so I withdraw that vote (in case anyone is counting!
As for my own spkg (eclib) I just make any change needed for Sage
"upstream" to keep life simple, but do realise that not all spkgs have
such accommodating owners!
John
John H Palmieri
> My entire proposal is:
>
> Modify "sage -pkg" so that it generates the patched files from the patches.
>
> That's it.
distribute the patches in order to make the spkg files smaller. I
don't know what effect this would actually have, but what's your
opinion about that?
--
John
William Stein
It would hardly make any difference in the size of the spkg files.
I don't think worrying about the savings one might get from this (a
few k) is worth the energy. One could surely get dozens of megs of
savings via other approaches. A whole bug days devoted to
"trimming the fat" in Sage would be good (e.g,. speed startup time,
shrink sage, etc.)
-- William
>
> --
> John
David Kirkby
> On Thu, Jul 1, 2010 at 12:05 PM, David Kirkby <david....@onetel.net> wrote:
>> I don't understand your proposal. Would it need the patch command
>> added to Sage? I don't understand your method, so can't comment
>> really.
>
> William's proposal is to
>
> 1) Standardize the filenames of patches so that the only file which
> patches ./src/foo/bar/xyz.py is ./patches/foo/bar/xyz.py.patch
>
> 2) Only the .patch file is checked into the repository.
>
> 3) When doing "sage -spkg" to create an spkg, it goes through all of
> the patches under ./patches/ and uses the patch command to make the
> patched file which is copied over. In the above example, the "sage
> -spkg" script would automatically make ./patches/foo/bar/xyz.py from
> ./src/foo/bar/xyz.py and ./patches/foo/bar/xyz.py.patch. Thus, only
> patch needs to be installed on the machine which creates the original
> spkg.
Many patches are currently applied conditionally - usually dependent
on the operating system of the system to which the Sage source will be
installed. One I can think of is applied only on the newer sun4v
systems like 't2' but not on older SPARC systems. But whether it gets
applied or not also depends on an environment variable.
Whilst applying patch as a result of an environment variable is not
common, applying them by operating system is very common.
I don't see how that situation could be handled by this method unless
we distribute a source for OS X, another for Linux, another for
Solaris ... etc etc.
Dave
Adam Webb
On Jul 2, 8:01 am, David Kirkby <david.kir...@onetel.net> wrote:
> On 1 July 2010 20:25, Mike Hansen <mhan...@gmail.com> wrote:
I don't know if there will really be a lot of space saved but I think
that is less important. The advantage of patches as I see it is that
it is a well known and used method of making changes to source code.
Having the original source and applying patches is the logical thing
to do.
The question seems to be when to apply the patches. Can the patches be
applied when the spkgs are made? Are there patches which are applied
conditionally at install time?
My personal preference is to patch as late as possible. When I get the
spkg I want the original source and not a patched version. Then I
still have the option of adding/removing/changing patches as needed
for my system. If I understand the proposal.
1. the diffs are made and included in the patches directory (details
of naming aside)
2. when sage -pkg is used the patched files are created
2b. conditional patches for specific systems and OS's could also
made (do we expect a lot of these?)
3. When the package is installed the patched files are copied over as
they are now
The bigger changes would be in the sage -pkg mechanism. The spkg-
install would also have to have conditions to copy the correct patches
for a specific system or OS but that is already the case.
The question then is if it is a burden to carry around the patches for
different systems. However, isn't this already the case?
Adam
drki...@gmail.com
> I felt that the important part of the proposal was the use of patches.
> I don't know if there will really be a lot of space saved but I think
>
> that is less important.
http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
and run all the self-tests in under 4 seconds.
> The advantage of patches as I see it is that
>
> it is a well known and used method of making changes to source code.
> Having the original source and applying patches is the logical thing
>
> to do.
>
>
>
> The question seems to be when to apply the patches. Can the patches be
>
> applied when the spkgs are made? Are there patches which are applied
>
> conditionally at install time?
>
> My personal preference is to patch as late as possible. When I get the
>
> spkg I want the original source and not a patched version. Then I
>
> still have the option of adding/removing/changing patches as needed
>
> for my system.
> If I understand the proposal.
>
> 1. the diffs are made and included in the patches directory (details
>
> of naming aside)
>
> 2. when sage -pkg is used the patched files are created
>
> 2b. conditional patches for specific systems and OS's could also
>
> made (do we expect a lot of these?)
It's a fact of life that any change has a non-zero probability of causing a bug, so it is often safer to make the patches specific to the system on which a problem is seen rather than to apply them everywhere.
Some things I'm aware of that affect whether a patch is applied or not include
* Operating system
* Whether the processor is Itanium or x86 on Linux.
* Whether the processor is SPARC on x86/x64 on Solaris.
* Whether the processor is the older sun4u or the more modern sun4v SPARC processor on Solaris systems.
* Whether the build is 32-bit or 64-bit
* Whether the linker is the Sun linker or the GNU linker.
* The value of an environment variable.
There may be other things which determine if a patch is applied or not.
> 3. When the package is installed the patched files are copied over as
>
> they are now
>
>
>
> The bigger changes would be in the sage -pkg mechanism. The spkg-
>
> install would also have to have conditions to copy the correct patches
>
> for a specific system or OS but that is already the case.
http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
together.
> The question then is if it is a burden to carry around the patches for
>
> different systems. However, isn't this already the case?
>
> Adam
drki...@gmail.com
> I don't know if there will really be a lot of space saved but I think
>
> that is less important.
http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
and run all the self-tests in under 4 seconds.
>
> it is a well known and used method of making changes to source code.
>
> to do.
>
>
>
> The question seems to be when to apply the patches. Can the patches be
>
> applied when the spkgs are made? Are there patches which are applied
>
> conditionally at install time?
>
>
> spkg I want the original source and not a patched version. Then I
>
> still have the option of adding/removing/changing patches as needed
>
> for my system.
>
> 1. the diffs are made and included in the patches directory (details
>
> of naming aside)
>
> 2. when sage -pkg is used the patched files are created
>
> 2b. conditional patches for specific systems and OS's could also
>
> made (do we expect a lot of these?)
It's a fact of life that any change has a non-zero probability of causing a bug, so it is often safer to make the patches specific to the system on which a problem is seen rather than to apply them everywhere.
Some things I'm aware of that affect whether a patch is applied or not include
* Operating system
* Whether the processor is Itanium or x86 on Linux.
* Whether the processor is SPARC on x86/x64 on Solaris.
* Whether the processor is the older sun4u or the more modern sun4v SPARC processor on Solaris systems.
* Whether the build is 32-bit or 64-bit
* Whether the linker is the Sun linker or the GNU linker.
* The value of an environment variable.
There may be other things which determine if a patch is applied or not.
>
> they are now
>
>
>
> The bigger changes would be in the sage -pkg mechanism. The spkg-
>
> install would also have to have conditions to copy the correct patches
>
> for a specific system or OS but that is already the case.
http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
together.
>
> different systems. However, isn't this already the case?
>
> Adam
Mike Hansen
conditional patches. I'm +1 on including GNU patch in Sage.
--Mike
Burcin Erocal
I also vote YES to including patch in Sage.
I would like to see some more standardization in how the patches are
created and applied though. Perhaps the Gentoo people can help here,
since the Gentoo build system provides wrappers to patch to eliminate
common errors:
http://devmanual.gentoo.org/ebuild-writing/functions/src_unpack/epatch/index.html
Cheers,
Burcin
François Bissey
Well yes, in ebuild we don't use the patch command directly. The wrapper
is part of an "eclass" (a collection of bash utilities, we have several of
these grouped by common tasks or functionality). The epatch wrapper
can even use compressed patches (not very useful for sage).
I am not sure a wrapper would be useful in sage but if it could save
repeating code or errors it could be worth it.
Francois
William Stein
> I don't think it's feasible to carry out William's proposal with
> conditional patches.
Why? Consider that we *already* successfully conditionally patch
files without using patch at all... and this works for every spkg
included in Sage.
> I'm +1 on including GNU patch in Sage.
>
> --Mike
>
Mike Hansen
> Why? Consider that we *already* successfully conditionally patch
> files without using patch at all... and this works for every spkg
> included in Sage.
Right, but it's all managed by hand. Once you start making automating
things, then they have to be standardized. For example, should every
patch get its own directory for the files that it touches? If A.patch
and B.patch both change setup.py, then you'll need to put those
patched setup.py files somewhere. What if you need the changes from
both A.patch and B.patch applied, would such a system automatically
make another version of setup.py with both those patches applied?
It seems like a way more complicated solution than what's needed.
We can also just have the patch.spkg "exit 0" if the patch is already installed.
--Mike
William Stein
I still vote -1 to this, and think it is possible to get around using
patch at runtime.
Nonetheless, I am ok with this proposal going forward, because it
clearly received a lot of support from most developers who commented.
William
Mike Hansen
> I still vote -1 to this, and think it is possible to get around using
> patch at runtime.
If we can do it and it's not too awkward, then that's great. But, I
personally am not sure how to implement it well. With using patch,
it's just very clear and explicit what is happening.
--Mike
Dr. David Kirkby
> Hi,
>
> I still vote -1 to this, and think it is possible to get around using
> patch at runtime.
> Nonetheless, I am ok with this proposal going forward, because it
> clearly received a lot of support from most developers who commented.
>
> William
OK
http://trac.sagemath.org/sage_trac/ticket/9418
is open to create the package with a target of Sage 5.0.
There are lots of tickets open now for updating spkg/standard/deps, so these
need to be coordinated. Actually adding the package will be pretty easy then.
One question I do ask, is whether the self-tests of the package should be run
automatically. If 'patch' fails to work properly, people are going to have very
broken builds of Sage. The time to install the package increased from 3.2 to 3.7
seconds on my Ultra 27, so I personally think adding 0.5 seconds to the install
time, and testing the 'patch' package is a good idea. In other words, we test it
on every installation, not just if someone has SAGE_CHECK=yes.
The package at http://boxen.math.washington.edu/home/kirkby/patches/patch-2.6.1.spkg
follows the conventional method of having a spkg-check file, and running the
self-tests only if SAGE_CHECK=yes. I think it would be wise that the package
runs the self-tests.
One problem of running the self-tests is that it would probably require 'diff'
exist on the system. I would have thought 'diff' existed on any system. It is
mandated by POSIX, but I've got no way of knowing if every version of Linux
comes with 'diff' or not.
Another related item is how to update the Sage documentation to reflect how to
make the patches. We need to tie down the exact method and naming convention for
patches.
http://trac.sagemath.org/sage_trac/ticket/9419
has been opened to update the Sage Developers Guide, once agreement is reached
on this. I will create another thread on sage-devel on how to actually create
the patch files as different people might well have different ideas on how to do
this. Let's keep that a separate thread.
Dave
Volker Braun
Then sage -pkg simply checks that either all patches in the queue are
applied or that there exists an old-style /patches directory and no
queue.
Complicated spkgs that require lots of modifications would then use a
patch queue. This is of course more complicated, but if your package
needs multiple patches then its probably not something than a new
developer can easily modify with the 'cp' method. For simple spkgs we
just keep the old process.
Either way, there would be no need for patch or mercurial at install
time.
Volker
Mike Hansen
> I would propose a mercurial patch queue in the spgk root directory.
> Then sage -pkg simply checks that either all patches in the queue are
> applied or that there exists an old-style /patches directory and no
> queue.
I think there are some issues with this:
1) The src/ directory needs be under Mercurial version control. This
would increase the size of the spkgs by quite a bit.
2) Many patches only need to be applied conditionally based on the
runtime environment. This can't happen with a queue unless Mercurial
is already installed. If we just use the patches/ directory when
there are conditionally applied patches, then we're in the same
situation as we are now.
--Mike
Volker Braun
> 1) The src/ directory needs be under Mercurial version control. This
> would increase the size of the spkgs by quite a bit.
in .hgignore and only selectively hg add the files that you actually
want to modify in your patch.
> 2) Many patches only need to be applied conditionally based on the
> runtime environment.
never become part of upstream. How hard is it to add an #ifdef bracket
around your patch?
Best wishes,
Volker
William Stein
> On Jul 3, 4:54 pm, Mike Hansen <mhan...@gmail.com> wrote:
>> 1) The src/ directory needs be under Mercurial version control. This
>> would increase the size of the spkgs by quite a bit.
>
> But you don't need to add all of src/. In fact, you could keep src
> in .hgignore and only selectively hg add the files that you actually
> want to modify in your patch.
True.
>
>> 2) Many patches only need to be applied conditionally based on the
>> runtime environment.
>
> I would argue that such patches are fundamentally flawed as they can
> never become part of upstream. How hard is it to add an #ifdef bracket
> around your patch?
>
That is also a very good point.
> Best wishes,
> Volker
Mike Hansen
>>> 2) Many patches only need to be applied conditionally based on the
>>> runtime environment.
>>
>> I would argue that such patches are fundamentally flawed as they can
>> never become part of upstream. How hard is it to add an #ifdef bracket
>> around your patch?
>
> That is also a very good point.
Here is an example patch for Python that is only applied on Itanium
Linux systems: http://sage.pastebin.com/1hy3cyis
Is there a non-autoconf way to have an ifdef to check for an Itanium
Linux system?
--Mike
Volker Braun
#ifdef __ia64__
// gcc itanium code here
#endif
Dr. David Kirkby
> I believe its
>
> #ifdef __ia64__
> // gcc itanium code here
> #endif
You would certainly want to add an 'ifdef linux' or similar, since Itanitium
processors can run FreeBSD, Windows, HP-UX and probably some other operating
systems too. (Sun started a port of Solaris once, but dropped the plans later).
Some patches even depend on the distribution of Linux - I know there is some
stuff only applied on openSUSE.
I think many patches in Sage are restricted to only one platform just to
simplify the testing and review process. Upstream package would no doubt be
better to apply the fix on every platform. But for Sage, if its not causing a
problem on Linux or OS X, but ony on Solaris, it will be much easier to get a
positive review if reviewers can see that it is only applied on platforms where
the problem is experienced.
I recall when iconv got added to Sage, it was done on every platform. Later it
was changed to be only Solaris and Cygwin when it became apparent having two
copies of iconv could cause problems on some Linux distributions.
Dave
Volker Braun
agree that, if the code is itanium linux-specific, then it must be
wrapped into
#if defined(__linux__) && ( defined(__ia64__)
// itanium linux specific
#endif
About the "readline on itanium" patch, wtf is going on? That patch is
just some random kludgery. Is http://trac.sagemath.org/sage_trac/ticket/488
the corresponding ticket? It doesn't even say what the bug is. There
is a sensible discussion on the python bugtracker of something that
sounds very related, see http://bugs.python.org/issue1204. There, you
can find an analysis of what went wrong and a correct fix that has,
moreover, long been incorporated into python. The correct fix is very
much different from what the sage patch mucks around with....
Volker
Dr. David Kirkby
> Does sage compile on anything but linux on itanium systems?
Probably not. But it would be shortsighted to write code that would make future
ports more difficult than necessary.
Several years ago, nobody had attempted to build Sage on 64-bit Solaris, so
someone decided to declare
typedef int int_fast32_t;
typedef long long int_fast64_t;
on just Solaris, which is valid for 32-bit code, but not 64-bit.
So I had to waste time sorting out their mistake.
http://trac.sagemath.org/sage_trac/ticket/9399
With a bit of foresight, they should have seen that this was not accurate.
(Solaris has been a 64-bit operating system for 15 years).
> But I
> agree that, if the code is itanium linux-specific, then it must be
> wrapped into
>
> #if defined(__linux__)&& ( defined(__ia64__)
> // itanium linux specific
> #endif
>
> About the "readline on itanium" patch, wtf is going on? That patch is
> just some random kludgery. Is http://trac.sagemath.org/sage_trac/ticket/488
> the corresponding ticket?
http://trac.sagemath.org/sage_trac/ticket/488
is about Readline, but does not mention openSUSE.
The readline patch I was talking about was the one which starts
if [ -f /etc/SuSE-release ]; then
if [ `grep 11.1 /etc/SuSE-release > /dev/null; echo $?` -eq 0 ]; then
echo "OpenSUSE 11.1 detected"
I've no idea if it's related to
http://bugs.python.org/issue1204
or not. I would not have associated them. #488 seems to be about readline, and
not specifically openSUSE.
> It doesn't even say what the bug is.
True, which is why I can't tell for 100% sure if
http://trac.sagemath.org/sage_trac/ticket/488
and
http://bugs.python.org/issue1204
are related or not. My gut feeling is they are unrelated.
> There
> is a sensible discussion on the python bugtracker of something that
> sounds very related, see http://bugs.python.org/issue1204. There, you
> can find an analysis of what went wrong and a correct fix that has,
> moreover, long been incorporated into python. The correct fix is very
> much different from what the sage patch mucks around with....
>
> Volker
If we implemented what I proposed the other day on sage-devel, under the title
"Suggestion to make reporting bugs upstream MANDATORY for Positive review"
http://groups.google.co.uk/group/sage-devel/browse_thread/thread/f84c603497d84485/d46bfa7052ea31e7
we might have more hope of being able to remove patches when bugs are fixed
upstream. If we did things the way I suggested,
http://trac.sagemath.org/sage_trac/ticket/488
would have a link to a bug reports on the OpenSUSE bug tracker
http://en.opensuse.org/Submitting_Bug_Reports
and the Python web site
It should be noted that
http://trac.sagemath.org/sage_trac/ticket/488
is a very old ticket, and Sage procedures have improved a lot since then. It
would be good if that progress could be maintained by putting even more emphasis
on code quality, and documenting things properly.
Dave
Volker Braun
wrote:
> http://trac.sagemath.org/sage_trac/ticket/488
> and
> http://bugs.python.org/issue1204
> are related or not. My gut feeling is they are unrelated.
http://groups.google.com/group/sage-devel/msg/b9fdd1c4c23c0161
mentions an immediate crash when using readline on itanium. The
analysis on
http://bugs.python.org/issue1204
is that readline on itanium crashes immediately when tab-completing
because of some symbol lookup issue. The correct fix is to modify
autotools to link to the correct readline. The (older) sage patch
lobotomizes the tab-completion code to achieve the same thing in a
highly non-portable way. Since it mucks around at the wrong place it
doesn't even collide with the patch for the correct fix.
Somebody with access to an itanium machine should try building sage
without the patch ;-)
Volker
Robert Bradshaw
> On Saturday, July 3, 2010, Volker Braun <vbrau...@gmail.com> wrote:
>> On Jul 3, 4:54 pm, Mike Hansen <mhan...@gmail.com> wrote:
>>> 1) The src/ directory needs be under Mercurial version control. This
>>> would increase the size of the spkgs by quite a bit.
>>
>> But you don't need to add all of src/. In fact, you could keep src
>> in .hgignore and only selectively hg add the files that you actually
>> want to modify in your patch.
>
> True.
I'm worried that it would be too easy to modify src and forget to add
the file. Currently, one can safely nuke src and drop in the pristine
upstream version.
>>> 2) Many patches only need to be applied conditionally based on the
>>> runtime environment.
>>
>> I would argue that such patches are fundamentally flawed as they can
>> never become part of upstream. How hard is it to add an #ifdef bracket
>> around your patch?
+1
I still lean towards including GNU patch--who is stepping up to
maintain the spkg for, say, the next two years (at least)?
- Robert
Carl Witty
> Then sage -pkg simply checks that either all patches in the queue are
> applied or that there exists an old-style /patches directory and no
> queue.
Since we all seem to like mercurial queues, we could also consider
using quilt (which could be thought of as "mercurial queues without
mercurial"). (Actually, I'm pretty sure that quilt came first, and
then mq based its command-line interface on quilt.) Debian, who has
the same sort of issue with upgrading packages that we do (times
several hundred!), uses quilt for many of their packages, and
recommends quilt in the "New Maintainers' Guide".
The revision-control portion of the workflow for updating a package in
the easiest case would go something like this:
1) extract spkg
2) rename directory to new version
3) remove old src/, unpack new source into src/
4) type: "quilt push -a"; make sure that all patches apply
5) type: "quilt pop -a"
6) edit SPKG.txt, check in changes to top-level repository
7) sage -pkg
The spkg-install would include: "quilt push -a"
If there's an error in step 4 (some old patch doesn't apply correctly
to the new source), then the procedure for fixing it is the same as
with queues: push patches one at a time with "quilt push"; when you
get to the patch that fails to apply, hand-apply it and do "quilt
refresh".
For cases where we have multiple variants, quilt supports multiple
series files; instead of "quilt push -a", it would be
"QUILT_SERIES=series.sun4v quilt push -a".
In the above variant of the proposal, we would need to include quilt
as an spkg. The source is a 420 kilobyte .tar.gz. On my desktop, it
takes 3.5 seconds to build (2 seconds to run configure, 1.5 seconds to
run make).
We could avoid shipping quilt as a standard spkg by rewriting "quilt
push -a" as our own shell script. (This is quite simple, and actually
recommended by the documentation. All the shell script needs to do is
take the series file and remove comments; then all that's left is a
list of patches to apply, one patch filename per line.) Then only
developers who want to work on spkgs including patches would need
quilt, so it could be an optional spkg (or we could require such
developers to just install quilt themselves).
We could avoid shipping either patch or quilt by going back to
William's suggestion at the top of the thread, where we automatically
make copies of the patched files at "sage -pkg" time, so that we can
copy the files at spkg build time without needing patch. With the
extra structure provided by quilt, with every variant represented by a
series.FOO file, I believe this would be straightforward even in the
case of multiple variants (although the multiple copies of patched
files would certainly make the spkg bigger).
I feel strongly (but have no hard evidence, of course) that the
problem of package updating with quilt would be much, much easier than
our current system, and also much easier than a system using patch
files in the case where the patches don't apply cleanly and a "quilt
refresh" is needed.
I'd be willing to put some effort into switching to quilt as the
recommended way to deal with patches in Sage, up to and including:
* make an spkg for quilt
* convert one spkg to using quilt, try to upgrade it with the above
workflow, and report on the process
* if people like the third variant, where we don't ship patch and
instead we automatically generate the patched versions of files at
spkg-build time, I'd be willing to write that tool
Carl
Robert Bradshaw
+1 I really like this idea. All the advantages of mercurial queues
without the late dependancies or requirement that the source be under
revision control. (The patch queue itself would be under the .spkg
control). The actual pushing of a series could probably be a simple
command in spkg-install, if we wanted to make the spkg optional.
That would be great.
- Robert
John Cremona
> +1 I really like this idea. All the advantages of mercurial queues
> without the late dependancies or requirement that the source be under
> revision control. (The patch queue itself would be under the .spkg
> control). The actual pushing of a series could probably be a simple
> command in spkg-install, if we wanted to make the spkg optional.
>
Me too. I have just been scrutinising the spkg-install for the pari
spkg and find that there are 10 files in patches/ which are applied as
follows:
* mostly they are just copied over the corresponding files in src/;
in some cases that deoends on the architecture
* one of them is plain copied if [ `uname` = "Linux" ] while if [
`uname` = "SunOS" ] then a sed script is applied to edit the original
* two of them (header files) are only replaced after pari is built
since the originals are needed to build it but changed files are
needed for the Sage library.
That is rather complicated (and I'm sure there are worse). And it is
a real pain to update the src -- one of those header files has changed
a lot even in the last 2 weeks since William and Robert made
pari-2.4.3.svn.p3 and so I need to do some manual editing to get their
changes into the p4 I am making; changes which I cannot test since
they are only needed on Darwin or SunOS.
In particular, any new system we put in place has to have conditional
application of patches depending on operating system.
John
Robert Bradshaw
Are there any packages where #ifders and/or environment-sensitive
makefiles would not do the job?
- Robert