Opened 7 years ago
Last modified 5 years ago
#14346 closed enhancement
The pari spkg is patching upstream too heavily — at Version 16
Reported by: | Snark | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | packages: standard | Keywords: | |
Cc: | jdemeyer | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The pari package has 15 patches, there are various problems with these:
- patches ..... should have been proposed upstream but haven't been reported.
- patch ..... breaks the ABI of the function ......(). It will cause problems when upgrading to the next PARI version because ........
polred.patch
in lines ... - ... of file ...... puts new developments in a package, which should instead be moved to the Sage library.
Change History (16)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
Both the library and the executable come out of the same spkg anyway so I don't see how the distinction matters...
comment:3 Changed 7 years ago by
Have to second that discusion on debian. I have actually pushed almost every single sage patch to pari in sage-on-gentoo. I think it is ok when they are patches included upstream. I hadn't looked at the particulars of polred. Which yes alter the ABI somewhat. Was the change necessary and will it be picked up upstream?
comment:4 Changed 7 years ago by
If you read B.A.'s comment on my debian bug report, I think it's 100% sure they won't take it in the stable branch, as it breaks ABI. Now for another branch, the question is open -- but since this polred.patch is based on upstream commits, they might have better up their sleeve already!
comment:5 Changed 7 years ago by
IIRC PARI 2.6 release sould happen quite soon, so hopefully the commits we used are from this devel branch, see http://pari.math.u-bordeaux1.fr/timeline.html
comment:6 follow-up: ↓ 8 Changed 7 years ago by
The commits used were from upstream, but the patch is based on those commits -- it's not just them. That means that the pari in sage is neither compatible with upstream's 2.5.3, nor with the upcoming 2.6.0.
So yes, we can hope that when sage will package pari 2.6.0, then it will stop forking pari and become compatible with something debian can package seriously.
comment:7 Changed 7 years ago by
- Cc jdemeyer added
comment:8 in reply to: ↑ 6 ; follow-up: ↓ 10 Changed 7 years ago by
Replying to Snark:
The commits used were from upstream, but the patch is based on those commits -- it's not just them. That means that the pari in sage is neither compatible with upstream's 2.5.3, nor with the upcoming 2.6.0.
I thought backporting patches from later upstream versions is standard practice.
comment:9 in reply to: ↑ description Changed 7 years ago by
- Milestone changed from sage-5.9 to sage-wishlist
- Type changed from defect to enhancement
Replying to Snark:
The pari package is full of patches
So what? Since when is patching a package a bug?
some of which should have been proposed upstream
Which patches are you refering to?
the correct solution would be for sage to *add* what is needed to the sage library instead of patching upstream.
Please elaborate how we should add PARI patches to the Sage library.
comment:10 in reply to: ↑ 8 ; follow-up: ↓ 12 Changed 7 years ago by
Replying to jdemeyer:
Replying to Snark:
The commits used were from upstream, but the patch is based on those commits -- it's not just them. That means that the pari in sage is neither compatible with upstream's 2.5.3, nor with the upcoming 2.6.0.
I thought backporting patches from later upstream versions is standard practice.
I'll have a shot at that one but it's probably nothing new to you. It depends on the trade off. Backporting in a distro like debian that is keeping fixed version of the software has much as possible is good.... so long as you preserve API. If the patch breaks API that means potentially all dependencies will need patching for the new API. So the trade off is between the number of dependencies you have to patch for the new API and how badly you want/need the backport.
It applies also to sage but the scale is a priori less than in a full distro. In the case of pari in particular, I don't think the list of dependencies (especially dependencies not in sage) affected by your change is very long (make that non existant until proven otherwise).
Debian may have enshrined the above discussion in a precise policy but I am not a deban-er.
comment:11 follow-up: ↓ 13 Changed 7 years ago by
Backporting is standard practice, to gain a bugfix or a new feature which is only available in a later version.
The current situation is different : some modifications are backports, and some are new code.
The backported patches have their places in the pari spkg, but the new code should be in the sage library.
comment:12 in reply to: ↑ 10 Changed 7 years ago by
Replying to fbissey:
If the patch breaks API
In which sense does the polred patch "break API" (I probably don't understand what you mean exactly with "breaks API")
comment:13 in reply to: ↑ 11 Changed 7 years ago by
Replying to Snark:
the new code should be in the sage library.
Which code are you refering too?
Details please, this is all so vague...
comment:14 follow-up: ↓ 15 Changed 7 years ago by
Ok, let's be explicit:
- in general there are fifteen patches in this spkg, which is way too much ;
- in particular polred.patch is a mix between a backport and a new development.
I claim putting new developments in a package is against standard packaging practice.
And I use the verb "break" because that's what will happen when sage will want to include the next pari version...
comment:15 in reply to: ↑ 14 Changed 7 years ago by
- Description modified (diff)
Replying to Snark:
Ok, let's be explicit:
That's not explicit at all, that's essentially just repeating what you already said. To make it easier for you, I changed the description and let you fill in the blanks.
comment:16 Changed 7 years ago by
- Description modified (diff)
Does this apply to the PARI library, or
gp
?(Of course
gp
uses the PARI library, but I think^{TM} the stand-alonegp
wouldn't need patching for Sage.)