Opened 3 years ago

Closed 3 years ago

#21604 closed enhancement (fixed)

Cleaning up stale installed files in setup()

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.4
Component: build Keywords:
Cc: mkoeppe Merged in:
Authors: Jeroen Demeyer Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 5ba95ed (Commits) Commit: 5ba95ed73aa3df532a2b6256510f2e2d842a0a35
Dependencies: #21480 Stopgaps:

Description

This part of src/setup.py should be moved inside some distutils command:

#########################################################
### Clean
#########################################################

print('Cleaning up stale installed files....')
t = time.time()
from sage_setup.clean import clean_install_dir
output_dirs = SITE_PACKAGES + glob.glob(os.path.join(SAGE_SRC, 'build', 'lib*'))
for output_dir in output_dirs:
    print('- cleaning {0}'.format(output_dir))
    clean_install_dir(output_dir, python_packages, python_modules,
            ext_modules, python_data_files)
print('Finished cleaning, time: %.2f seconds.' % (time.time() - t))

Change History (15)

comment:1 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/cleaning_up_stale_installed_files_in_setup__

comment:2 Changed 3 years ago by jdemeyer

  • Commit set to 274d8b378586814a35d67ce342192f617bcbe97e
  • Status changed from new to needs_review

New commits:

274d8b3Clean up stale installed files in install command

comment:3 Changed 3 years ago by mkoeppe

Could you add a few lines of comments to the patch that explain what "stale installed files" are?

comment:4 Changed 3 years ago by git

  • Commit changed from 274d8b378586814a35d67ce342192f617bcbe97e to e50f978de0967be95d76534f7e9a5cfd740f8008

Branch pushed to git repo; I updated commit sha1. New commits:

e50f978Add some documentation

comment:5 Changed 3 years ago by jdemeyer

  • Dependencies set to #21580
  • Status changed from needs_review to needs_work

comment:6 Changed 3 years ago by jdemeyer

  • Dependencies changed from #21580 to #21480

comment:7 Changed 3 years ago by git

  • Commit changed from e50f978de0967be95d76534f7e9a5cfd740f8008 to 5ba95ed73aa3df532a2b6256510f2e2d842a0a35

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

751bd0fReword TODO item
3a8cc0eFix typo in comment
0dd9c50Respect environment variable MAKE
17f90d8beautification
e5f9065More comments
7791cd9Remove --buildbase code
74169e7Pass SAGE_SRC to generate_py_source.mk
0394333Add new file to MANIFEST.in
fdedb02Install SAGE_LOCAL/bin/sage instead of SAGE_ROOT/sage as Jupyter kernel
5ba95edClean up stale installed files in install command

comment:8 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Rebased on top of #21480.

comment:9 Changed 3 years ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

comment:10 follow-up: Changed 3 years ago by embray

  • Status changed from positive_review to needs_work

As I wrote in #21600 this makes sense, but I think this should happen prior to any "build" command, not just at install time, since this is really an issue of keeping around a build/ dir from a previous build and expecting to be able to reuse it for a new build. I can appreciate the need for that, I would just move this step earlier, maybe even as a sub-command of build. If you don't mind, I can update this with what I have in mind.

comment:11 in reply to: ↑ 10 Changed 3 years ago by jdemeyer

Replying to embray:

As I wrote in #21600 this makes sense, but I think this should happen prior to any "build" command, not just at install time, since this is really an issue of keeping around a build/ dir from a previous build and expecting to be able to reuse it for a new build. I can appreciate the need for that, I would just move this step earlier, maybe even as a sub-command of build. If you don't mind, I can update this with what I have in mind.

Couldn't this be done as follow-up ticket? Otherwise we will never make progress.

What you say makes sense, but the changes made in this ticket also make sense.

comment:12 Changed 3 years ago by mkoeppe

I agree with Jeroen. This ticket is an important step, moving the "clean" part inside of setup. So, for example, setup.py --help no longer invokes cleaning. A follow-up ticket (please make one) should disentangle the cleaning of the build directory and the cleaning of the install directory.

comment:13 Changed 3 years ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:14 Changed 3 years ago by mkoeppe

I've created the follow-up ticket at #21654.

comment:15 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/cleaning_up_stale_installed_files_in_setup__ to 5ba95ed73aa3df532a2b6256510f2e2d842a0a35
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.