Opened 3 years ago

Closed 3 years ago

#23972 closed enhancement (fixed)

Do not delete non-matching tarball in upstream/

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.1
Component: packages: standard Keywords:
Cc: jdemeyer Merged in:
Authors: Jeroen Demeyer Reviewers: Vincent Klein
Report Upstream: N/A Work issues:
Branch: 8c72d8e (Commits) Commit: 8c72d8ebc0656c396ce25f853b34d232eb397000
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

If a tarball does not match the checksums, it is deleted by sage -i. This is very confusing. A warning is shown, but it's easy to miss.

We see this over an over again, e.g. on #23898.

Change History (11)

comment:1 Changed 3 years ago by embray

+1 I have also been bitten by this. I don't see why it should be deleted as long as it isn't installed.

comment:2 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from do not silently delete non-matching tarball in upstream/ to Do not delete non-matching tarball in upstream/

First of all, it's not silently deleted, there is a message: 

[zlib-1.2.11] Found local metadata for zlib-1.2.11
[zlib-1.2.11] Invalid checksum for cached file /usr/local/src/sage-config/upstream/zlib-1.2.11.tar.gz, deleting

I think the auto-deleting was implemented by Volker to make things easier for bots. If a bot has a wrong tarball in upstream for some reason, then it's easiest to simply re-download it.

Maybe the best solution would be to only delete the existing tarball when a correct one was successfully downloaded.

comment:3 Changed 3 years ago by vbraun

If the bots don't auto-delete files with incorrect checksums then it'll be easy to foobar all patchbots with one incorrect ticket...

comment:4 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/do_not_delete_non_matching_tarball_in_upstream_

comment:5 Changed 3 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to 8c72d8ebc0656c396ce25f853b34d232eb397000
  • Status changed from new to needs_review

I think this solution works: do not delete the bad tarball, just ignore it and download a new version replacing the bad version.

If the tarball does exist on the mirrors, the result should be the same as now. If the tarball does not exist on the mirrors, it is simply left in place.

New commits:

​8c72d8eDo not delete tarballs with wrong checksums

comment:6 follow-up: Changed 3 years ago by vklein

Note that your commit doesn't seems to solve the case encountered in #23898. Your tarball is not deleted but is crushed by the following download.

I still think this ticket is usefull when you are upgrading a package version or creating a new package.

Last edited 3 years ago by vklein (previous) (diff)

comment:7 Changed 3 years ago by vklein

  • Reviewers set to Vincent Klein

comment:8 Changed 3 years ago by vklein

  • Status changed from needs_review to positive_review

It works. Tests have been done on top of sage 8.3.beta6 version

comment:9 in reply to: ↑ 6 Changed 3 years ago by jdemeyer

Replying to vklein:

Note that your commit doesn't seems to solve the case encountered in #23898.

What precisely is "the case encountered in #23898"?

comment:10 Changed 3 years ago by vklein

I am refering to that ​https://trac.sagemath.org/ticket/23898#comment:22

My point was that the package replacement is as "silent" as before with this ticket. But reading again ​comment 2 you're well aware of that.

comment:11 Changed 3 years ago by vbraun

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