8000 remove patch spkg by dimpase · Pull Request #39943 · sagemath/sage · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

remove patch spkg #39943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 11, 2025
Merged

remove patch spkg #39943

merged 1 commit into from
May 11, 2025

Conversation

dimpase
Copy link
Member
@dimpase dimpase commented Apr 15, 2025

every system we support has a decent enough version of patch. So we just purge it, like we did with tar a while ago.

This will fix #39941 and #30481

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@dimpase
Copy link
Member Author
dimpase commented Apr 15, 2025

@jhpalmieri @kiwifb - on a closer inspection, we can just put patch into pre-reqs, and purge the spkg.

@kiwifb
Copy link
Member
kiwifb commented Apr 15, 2025

That's acceptable to me.

@github-actions GitHub Actions
Copy link
github-actions bot commented Apr 15, 2025 •
edited
Loading

Documentation preview for this PR (built with commit eebe303; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@dimpase
Copy link
Member Author
dimpase commented Apr 15, 2025

there are some docker-related failures in doctests, but tests do pass otherwise. @kiwifb @jhpalmieri - positive review?

@jhpalmieri
Copy link
Member

Do you know why we have been using the argument --no-backup-if-mismatch?

@dimpase
Copy link
Member Author
dimpase commented Apr 15, 2025

something from pre-historical patchquilt era?

@kiwifb
Copy link
Member
kiwifb commented Apr 15, 2025

Do you know why we have been using the argument --no-backup-if-mismatch?

My first reaction was "not to leave litter everywhere" but building packages is done in temporary folders so the rubbish is all properly collected.

@jhpalmieri
Copy link
Member

Do you know why we have been using the argument --no-backup-if-mismatch?

My first reaction was "not to leave litter everywhere" but building packages is done in temporary folders so the rubbish is all properly collected.

What about a package like boost_cropped? Its spkg-install file says
sdh_install src/boost "$SAGE_LOCAL/include"
which copies the entire directory src/boost, so if some file happened to be created there by patch, it would get copied over to Sage. How do we guarantee that something like this doesn't happen?

@dimpase
Copy link
Member Author
dimpase commented Apr 16, 2025

How do we guarantee that something like this doesn't happen?

if patch fails (on mismatch) then the installation is aborted. So no, nothing gets copied anywhere.

@dimpase dimpase requested a review from tobiasdiez April 18, 2025 17:56
@dimpase
Copy link
Member Author
dimpase commented Apr 18, 2025

@vbraun - any possible issues with the release manager workflows here?

tobiasdiez
tobiasdiez approved these changes Apr 19, 2025
View reviewed changes
Copy link
Contributor
@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Feel free to toggle to positive review (if others don 8000 't have any further remarks).

@dimpase dimpase mentioned this pull request Apr 19, 2025
Merged
5 tasks
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 21, 2025
dc66509 
sagemathgh-39943: remove patch spkg
    
every system we support has a decent enough version of patch. So we just
purge it, like we did with tar a while ago.

This will fix sagemath#39941 and sagemath#30481

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39943
Reported by: Dima Pasechnik
Reviewer(s): Tobias Diez
@dimpase
remove patch spkg
eebe303 
every system we support has a decent enough version of patch.
So we just purge it, like we did with tar a while ago.
@dimpase
Copy link
Member Author
dimpase commented Apr 30, 2025

@vbraun - this is an important PR - without it people on the new patch (2.8.*) getting to build patch from source, as patch 2.8 reports a 2-digit version we don't grok. Please give it a prioriy

@dimpase dimpase mentioned this pull request Apr 30, 2025
Merged
5 tasks
vbraun pushed a commit to vbraun/sage that referenced this pull request May 1, 2025
1901f40 
sagemathgh-39943: remove patch spkg
    
every system we support has a decent enough version of patch. So we just
purge it, like we did with tar a while ago.

This will fix sagemath#39941 and sagemath#30481

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39943
Reported by: Dima Pasechnik
Reviewer(s): Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request May 4, 2025
3d7d360 
sagemathgh-39943: remove patch spkg
    
every system we support has a decent enough version of patch. So we just
purge it, like we did with tar a while ago.

This will fix sagemath#39941 and sagemath#30481

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39943
Reported by: Dima Pasechnik
Reviewer(s): Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request May 4, 2025
4a92b26 
sagemathgh-40033: Singular 4.4.1 + Flint 3.3.2
    
Update flint and singular.
This will catch up with Conda and Homebrew, possibly other distros

(Homebrew needs Homebrew/homebrew-core#222142
merged)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

sagemath#39943: - just not to build patch from source (if you have systemwide
patch 2.8)

sagemath#39977: - patches to allow gcc-15, etc
    
URL: sagemath#40033
Reported by: Dima Pasechnik
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request May 5, 2025
64b2e02 
sagemathgh-39943: remove patch spkg
    
every system we support has a decent enough version of patch. So we just
purge it, like we did with tar a while ago.

This will fix sagemath#39941 and sagemath#30481

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39943
Reported by: Dima Pasechnik
Reviewer(s): Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request May 5, 2025
b748cf5 
sagemathgh-40033: Singular 4.4.1 + Flint 3.3.2
    
Update flint and singular.
This will catch up with Conda and Homebrew, possibly other distros

(Homebrew needs Homebrew/homebrew-core#222142
merged)
EDIT Homebrew/homebrew-core#222142 has been
merged, so now you can install singular from Homebrew and use it!

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

sagemath#39943: - just not to build patch from source (if you have systemwide
patch 2.8)

sagemath#39977: - patches to allow gcc-15, etc
    
URL: sagemath#40033
Reported by: Dima Pasechnik
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request May 6, 2025
23bd37f 
sagemathgh-39943: remove patch spkg
    
every system we support has a decent enough version of patch. So we just
purge it, like we did with tar a while ago.

This will fix sagemath#39941 and sagemath#30481

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [x] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39943
Reported by: Dima Pasechnik
Reviewer(s): Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this pull request May 6, 2025
5396269 
sagemathgh-40033: Singular 4.4.1 + Flint 3.3.2
    
Update flint and singular.
This will catch up with Conda and Homebrew, possibly other distros

(Homebrew needs Homebrew/homebrew-core#222142
merged)
EDIT Homebrew/homebrew-core#222142 has been
merged, so now you can install singular from Homebrew and use it!

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

sagemath#39943: - just not to build patch from source (if you have systemwide
patch 2.8)

sagemath#39977: - patches to allow gcc-15, etc
    
URL: sagemath#40033
Reported by: Dima Pasechnik
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request May 9, 2025
c46e0a2 
sagemathgh-40033: Singular 4.4.1 + Flint 3.3.2
    
Update flint and singular.
This will catch up with Conda and Homebrew, possibly other distros

(Homebrew needs Homebrew/homebrew-core#222142
merged)
EDIT Homebrew/homebrew-core#222142 has been
merged, so now you can install singular from Homebrew and use it!

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

sagemath#39943: - just not to build patch from source (if you have systemwide
patch 2.8)

sagemath#39977: - patches to allow gcc-15, etc
    
URL: sagemath#40033
Reported by: Dima Pasechnik
Reviewer(s):
@vbraun vbraun merged commit 4676554 into sagemath:develop May 11, 2025
27 of 29 checks passed
@dimpase dimpase mentioned this pull request May 30, 2025
Merged
5 tasks
@dimpase
Copy link
Member Author
dimpase commented Jun 1, 2025

I left inst_patch in base target of Makefile.in, this should be fixed. I noticed this while working on a followup, removing pkgconf spkg - after the latter is done, base target will go, as it's only (potentially) installing pkgconf now.

@dimpase dimpase deleted the patchtogo branch June 1, 2025 02:43
dimpase added a commit to dimpase/sage that referenced this pull request Jun 1, 2025
@dimpase
remove leftover from sagemath#39943
6949506 
inst_patch no longer even defined, so we should remove it from the deps of the base target
dimpase added a commit to dimpase/sage that referenced this pull request Jun 6, 2025
@dimpase
remove leftover from sagemath#39943
f15839d 
inst_patch no longer even defined, so we should remove it from the deps of the base target
mkoeppe added a commit to passagemath/passagemath that referenced this pull request Jun 17, 2025
@mkoeppe
Unmerge sagemath/sage#39943
dd30497
mkoeppe added a commit to passagemath/passagemath that referenced this pull request Jun 17, 2025
@mkoeppe
Unmerge sagemath/sage#39943
4febc96
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobiasdiez tobiasdiez tobiasdiez approved these changes

Assignees
No one assigned
Labels
c: packages: standard p: blocker / 1 p: critical / 2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2-digit version of GNU patch is not recognised
5 participants
@dimpase @kiwifb @jhpalmieri @tobiasdiez @vbraun
0