Skip to content

Commit

Permalink
sagemathgh-37352: sage --package create: When re-creating with a di…
Browse files Browse the repository at this point in the history
…fferent source type, clean up

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
For example, when switching from a `pip` package to `wheel` package,
remove the `requirements.txt` file.
When switching from `normal` to `wheel`, remove the `spkg-install.in`
file.

One of these cases was handled by a commit on sagemath#36641, cherry-picked from
there.
Generalizing here to avoid mistakes such as
sagemath#37129 (comment) in
the future.

Example:
```
$ ./sage -package create imagesize --pypi
Downloading tarball from https://pypi.io/packages/py2.py3/i/imagesize/im
agesize-1.4.1-py2.py3-none-any.whl to /Users/mkoeppe/s/sage/sage-
rebasing/worktree-pristine/upstream/imagesize-1.4.1-py2.py3-none-any.whl
[......................................................................]
$ git status
On branch sage_package_create_remove_files
Your branch is ahead of 'upstream/develop' by 3 commits.
  (use "git push" to publish your local commits)

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   build/pkgs/imagesize/SPKG.rst
        modified:   build/pkgs/imagesize/checksums.ini
        modified:   build/pkgs/imagesize/install-requires.txt
        deleted:    build/pkgs/imagesize/spkg-install.in
        modified:   build/pkgs/imagesize/type
```

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] 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 accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#37352
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
  • Loading branch information
Release Manager committed Feb 19, 2024
2 parents 67c6dfc + 7f1d531 commit 06625e2
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
24 changes: 23 additions & 1 deletion build/sage_bootstrap/creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ def heading(title, char='-'):
if upstream_contact:
f.write('{0}\n\n'.format(upstream_contact))

def _remove_files(self, files):
"""
Remove ``files`` from the package directory if they exist.
"""
for file in files:
try:
os.remove(os.path.join(self.path, file))
except OSError:
pass

def set_python_data_and_scripts(self, pypi_package_name=None, source='normal'):
"""
Write the file ``dependencies`` and other files for Python packages.
Expand All @@ -90,6 +100,8 @@ def set_python_data_and_scripts(self, pypi_package_name=None, source='normal'):
If ``source`` is ``"wheel"``, write the file ``install-requires.txt``.
If ``source`` is ``"pip"``, write the file ``requirements.txt``.
Remove existing files that belong to other source types.
"""
if pypi_package_name is None:
pypi_package_name = self.package_name
Expand All @@ -101,13 +113,23 @@ def set_python_data_and_scripts(self, pypi_package_name=None, source='normal'):
f.write('cd src\nsdh_pip_install .\n')
with open(os.path.join(self.path, 'install-requires.txt'), 'w+') as f:
f.write('{0}\n'.format(pypi_package_name))
# Remove this file, which would mark the package as a pip package.
self._remove_files(['requirements.txt'])
elif source == 'wheel':
with open(os.path.join(self.path, 'install-requires.txt'), 'w+') as f:
f.write('{0}\n'.format(pypi_package_name))
# Remove this file, which would mark the package as a pip package.
self._remove_files(['requirements.txt'])
if pypi_package_name != 'pip':
# 'pip' should be the only wheel package that has a custom spkg-install.in script.
# Remove the script for all other wheel packages, to avoid errors when
# switching from normal to wheel packages.
self._remove_files(['spkg-build.in', 'spkg-install.in', 'spkg-install'])
elif source == 'pip':
with open(os.path.join(self.path, 'requirements.txt'), 'w+') as f:
f.write('{0}\n'.format(pypi_package_name))
self._remove_files(['checksums.ini', 'spkg-build.in', 'spkg-install.in', 'spkg-install', 'install-requires.txt'])
elif source == 'script':
pass
self._remove_files(['checksums.ini', 'requirements.txt'])
else:
raise ValueError('package source must be one of normal, script, pip, or wheel')
4 changes: 4 additions & 0 deletions src/doc/en/developer/packaging.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ the following source types:

- its version number is defined by the required file ``package-version.txt``;

- no build and install scripts are needed
(with one exception: the package :ref:`spkg_pip` installs itself from
its wheel using a custom install script);

- Sage records the version number of the package installed using a file in
``$SAGE_LOCAL/var/lib/sage/installed/`` and will rerun the installation
if ``package-version.txt`` changes.
Expand Down

0 comments on commit 06625e2

Please sign in to comment.