Opened 4 years ago

Closed 4 years ago

#18566 closed enhancement (fixed)

Warning when installing new-style experimental packages

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.8
Component: packages: experimental Keywords:
Cc: vbraun, jdemeyer Merged in:
Authors: Nathann Cohen Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 0d344a3 (Commits) Commit: 0d344a334c0cfbb25524c59ce0205004762030d2
Dependencies: #18563 Stopgaps:

Description

With #18563, which changes the status of several optional new-style packages to 'experimental', we now need to display a warning when these scripts are installed.

Nathann

Change History (15)

comment:1 Changed 4 years ago by ncohen

  • Branch set to public/18566
  • Status changed from new to needs_review

comment:2 Changed 4 years ago by git

  • Commit set to b4d152bc305eed215af74a0c51c39043ce781a8d

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

ef11087Change known-broken packages to "experimental"
37fd689Valgrind doesn't build on OS X 10.10
b4d152btrac #18566: Warning when installing new-style experimental packages

comment:3 follow-up: Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

I would change

if [ `cat "$PKG_SCRIPTS/type"` == "experimental" ]

to

if [ x`cat "$PKG_SCRIPTS/type"` = xexperimental ]

which has less chances of trouble (in case of a missing/empty type file for example).

comment:4 follow-up: Changed 4 years ago by jdemeyer

Instead of

echo # The following message appears twice in this file

couldn't you just make a function warn_experimental() which displays that message?

comment:5 Changed 4 years ago by git

  • Commit changed from b4d152bc305eed215af74a0c51c39043ce781a8d to 6cafb514f9b9487223b5e2eb2b76fafe0a00661d

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

6cafb51trac #18566: Bugfix

comment:6 in reply to: ↑ 3 Changed 4 years ago by ncohen

I would change

if [ `cat "$PKG_SCRIPTS/type"` == "experimental" ]

to

if [ x`cat "$PKG_SCRIPTS/type"` = xexperimental ]

Nice. I did not know that corner-case, not that workaround. Done.

comment:7 Changed 4 years ago by jdemeyer

= instead of == in shell scripts (in POSIX, only = is supported, bash happens to support == also)

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:8 in reply to: ↑ 4 Changed 4 years ago by ncohen

Yo !

Instead of

echo # The following message appears twice in this file

couldn't you just make a function warn_experimental() which displays that message?

I first did, then decided against it for the following reason: there is one message for experimental packages (appears twice) and one message for archived packages (appears once). As for old-style packages the two messages follow each other, it seemed weird to have one called from a function and another one given explicitly. Plus It would have meant that the new functions would have to be defined in the middle of unrelated code, somewhere else.

So well, I found it cleaner this way.

Nathann

comment:9 Changed 4 years ago by git

  • Commit changed from 6cafb514f9b9487223b5e2eb2b76fafe0a00661d to 40a7807458f1fc890db4f9ec0b50b99f4724e94d

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

40a7807trac #18566: Bugfix

comment:10 Changed 4 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:11 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Please rebase to sage-6.8.beta3

comment:12 Changed 4 years ago by git

  • Commit changed from 40a7807458f1fc890db4f9ec0b50b99f4724e94d to 0d344a334c0cfbb25524c59ce0205004762030d2

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

0d344a3trac #18566: Merged with 6.8.beta3

comment:13 Changed 4 years ago by ncohen

  • Status changed from needs_work to needs_review

comment:14 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:15 Changed 4 years ago by vbraun

  • Branch changed from public/18566 to 0d344a334c0cfbb25524c59ce0205004762030d2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.