Opened 4 years ago

Closed 4 years ago

#18820 closed enhancement (fixed)

Upgrade R to 3.2.1

Reported by: charpent Owned by:
Priority: major Milestone: sage-6.8
Component: packages: standard Keywords: r-project
Cc: Merged in:
Authors: Emmanuel Charpentier Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: b46fa1a (Commits) Commit: b46fa1a5b3d462e89223e0cdf37d84c67d483e35
Dependencies: Stopgaps:

Description (last modified by charpent)

This minor update (mostly bugfixes) deserves admission in R in order to be able to get support on R-help.

Original tarball here (too fat to be attached).

Change History (29)

comment:1 Changed 4 years ago by charpent

  • Branch set to u/charpent/upgrade_r_to_3_2_1

comment:2 Changed 4 years ago by charpent

  • Commit set to 9a9c4ccd78dda128da41df76089fb87462a870a4
  • Description modified (diff)
  • Status changed from new to needs_review

Used the new convention allowing capitalized tarball names.

Passes ptestlong on Debian testing.

comment:3 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Where is the tarball?...

comment:4 Changed 4 years ago by charpent

  • Description modified (diff)
  • Status changed from needs_info to needs_review

<Wups !> Forgot the (link to) the tarball... Fixed.

comment:5 follow-up: Changed 4 years ago by ncohen

Why did you remove the spkg-src file?

comment:6 in reply to: ↑ 5 ; follow-up: Changed 4 years ago by charpent

Replying to ncohen:

Dear Nathaan,

Why did you remove the spkg-src file?

You advised (see #16711 about upgrade to 3.1.1) adding this file to document the renaming of the tarball (from R-x-y-z.tar.gz to r-x-y-z.tar.gz), needed by the then-current convention of all-lowercase tarball names.

Since this convention has been obsoleted (see #18229 for the previous upgrade to 3.2.0), this renaming is no longer necessary and one can use the original tarball. There is nothing to document. Since, according to the developer's guide, this file is optional, it becomes superfluous. QED.

Do you advise keeping a no-op spkg-src ?

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

comment:7 in reply to: ↑ 6 ; follow-up: Changed 4 years ago by ncohen

Hellooooooo,

You advised (see #16711 about upgrade to 3.1.1) adding this file to document the renaming of the tarball (from R-x-y-z.tar.gz to r-x-y-z.tar.gz), needed by the then-current convention of all-lowercase tarball names.

A spkg-src file to document a renaming? Why? O_o

Do you advise keeping a no-op spkg-src ?

No. I advise to keep a file that re-builds the archive from network ressources only. Then it becomes easier to check that the archive's content is what it should be. I do not see anything wrong with a script that downloads a distant file (whose url is generated from the locally stored version number), and does nothing else to it if there is nothing else to do.

Actually, I will try to solve this once and for all. I'll write some 'sage --sanity-check-package whatever' that uses whatever's spkg-src script (if any), and checks whether the content of the file in upstream/ matches it.

This way, checking that the archive is legit will only be a call to 'sanity-check'. In this setting, your spkg-src, even though it just downloads something, will be useful.

Nathann

comment:8 in reply to: ↑ 7 ; follow-up: Changed 4 years ago by charpent

Replying to ncohen:

Hellooooooo,

You advised (see #16711 about upgrade to 3.1.1) adding this file to document the renaming of the tarball (from R-x-y-z.tar.gz to r-x-y-z.tar.gz), needed by the then-current convention of all-lowercase tarball names.

A spkg-src file to document a renaming? Why? O_o

Do you advise keeping a no-op spkg-src ?

No. I advise to keep a file that re-builds the archive from network ressources only.

Please don't do that (unless you have a solid reason to do so) : some firewall setup do exceedingly annoying thing to network downloads. The recent "enhancement" to downloads that tries to select the closest/fastest mirror left me with an unupdateable tree (and, yes, this machines lives behind a "corporate-style" fascist firewall, run by incompetents...). I didn't yet had time to figure out how to unwedge it in order to test your (and vbraun's) solution)....

Then it becomes easier to check that the archive's content is what it should be. I do not see anything wrong with a script that downloads a distant file (whose url is generated from the locally stored version number), and does nothing else to it if there is nothing else to do.

Actually, I will try to solve this once and for all. I'll write some 'sage --sanity-check-package whatever' that uses whatever's spkg-src script (if any), and checks whether the content of the file in upstream/ matches it.

This way, checking that the archive is legit will only be a call to 'sanity-check'. In this setting, your spkg-src, even though it just downloads something, will be useful.

Again, ponder what you're planning. Im currently in a hell paved of yur good intentins.

Whatever you decide to do, could you please open a SEPARATE ticket, instead of hijacking this innocent R upgrade YET AGAIN (that would be the fourth time...) ? Thanks in advance !

comment:9 in reply to: ↑ 8 ; follow-ups: Changed 4 years ago by ncohen

  • Status changed from needs_review to needs_work

Please don't do that (unless you have a solid reason to do so) : some firewall setup do exceedingly annoying thing to network downloads.

How are the two connected? Nobody will be forced to run the script I just wrote.

Again, ponder what you're planning. Im currently in a hell paved of yur good intentins.

Whatever you decide to do, could you please open a SEPARATE ticket, instead of hijacking this innocent R upgrade YET AGAIN (that would be the fourth time...) ? Thanks in advance !

I had no intention to do it here. I was about to mail to sage-devel, as this is a much larger change. What I will do here, however, is ask you to not remove the spkg-src script.

Nathann

comment:10 in reply to: ↑ 9 ; follow-up: Changed 4 years ago by ncohen

Whatever you decide to do, could you please open a SEPARATE ticket, instead of hijacking this innocent R upgrade YET AGAIN (that would be the fourth time...) ? Thanks in advance !

Please provide the list of the other tickets that you think I hijacked.

Nathann

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

comment:11 in reply to: ↑ 10 ; follow-up: Changed 4 years ago by charpent

Replying to ncohen:

Whatever you decide to do, could you please open a SEPARATE ticket, instead of hijacking this innocent R upgrade YET AGAIN (that would be the fourth time...) ? Thanks in advance !

Please provide the list of the other tickets that you think I hijacked.

Whoa ! I didn't say YOU hijacked them. Apologies for loose redaction.

I said that R upgrades (which should be very plain vanilla affairs) seem to attract delays from unrelated issues. From memory (not real time to dig them out)

  • Upgrade to 3.2.0 was hijacked by the lowercase-tarball-convention reversing
  • A couple of versions before, hijacking for cygnus-building issues
  • [ I don't have the exact third issue on hand nor on memory, but I know that the Cignus-related one wasn't the first. I'll have to look with a bit more time ]

So your use of a previously unused spkg-src file would have been the fourth time...

Emmanuel Charpentier

comment:12 in reply to: ↑ 9 ; follow-up: Changed 4 years ago by charpent

Replying to ncohen:

Please don't do that (unless you have a solid reason to do so) : some firewall setup do exceedingly annoying thing to network downloads.

How are the two connected? Nobody will be forced to run the script I just wrote.

Again, ponder what you're planning. Im currently in a hell paved of yur good intentins.

Whatever you decide to do, could you please open a SEPARATE ticket, instead of hijacking this innocent R upgrade YET AGAIN (that would be the fourth time...) ? Thanks in advance !

I had no intention to do it here. I was about to mail to sage-devel, as this is a much larger change. What I will do here, however, is ask you to not remove the spkg-src script.

Okay. I'll post a new commit tonight (can't do that now : I'm at work, and I prepare tickets on a home machine).

I intend to discuss your proposal seriously : it entails creating a spkg-src file for all packages, thus making it mandatory. And making mandatory the use of a functional Internet connection for any installation/update. No more updates for an isolated/firewalled machine...

And since we pay this price, it should be logical to limit Sage tarballs to a very bare minimum of sage-specific files and patches.

I'm not sure that's where we should go... But this is the subject of your proposal-to-be, that will be discussed on its ticket.

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

Whoa ! I didn't say YOU hijacked them. Apologies for loose redaction.

Oh. I did misread it a bit too, apparently. Sorry.

Nathann

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

I intend to discuss your proposal seriously : it entails creating a spkg-src file for all packages, thus making it mandatory. And making mandatory the use of a functional Internet connection for any installation/update. No more updates for an isolated/firewalled machine...

Nonono, it is much more innocent than that. I just want to have some script that reviewers can run whenever they review a package, to make sure everything is fine. I don't plan to make it automatic at all. Just a helper code for reviewers.

And since we pay this price, it should be logical to limit Sage tarballs to a very bare minimum of sage-specific files and patches.

It would help with that, for sure. There have been recent complaints about how we sometimes change upstream's package, and how it is not always said explicitly.

I'm not sure that's where we should go... But this is the subject of your proposal-to-be, that will be discussed on its ticket.

On #18826, right.

Nathann

comment:15 Changed 4 years ago by git

  • Commit changed from 9a9c4ccd78dda128da41df76089fb87462a870a4 to 769fc49bba2c1508cc61df75d91f4ea9c7214965

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

769fc49Restored spkg-src.

comment:16 Changed 4 years ago by charpent

  • Status changed from needs_work to needs_review

Nathan,

Voluntatem tuam satisfeci. Hoc super caput tuum requiet.

Emmanuel Charpentier

Version 1, edited 4 years ago by charpent (previous) (next) (diff)

comment:17 Changed 4 years ago by ncohen

Hello again,

In its new version, your branch removes *one* line from the spkg-src. It is not that I care much about this line, but rather that the next one has absolutely no meaning: you removed the instruction that moves the archive to upstream/, but left the one which updates the pacakges' checksums using the information from upstream/.

Could you sort this out (in the way that you prefer)? You can then set this ticket to positive_review.

THaaaaaaaaaanks,

Nathann

comment:18 Changed 4 years ago by ncohen

  • Reviewers set to Nathann Cohen

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

Given that you're not actually using the spkg-src script, why don't you just remove it?

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

Given that you're not actually using the spkg-src script, why don't you just remove it?

He did, and I asked him not to. You can tell from the history of this ticket.

I asked him to keep that file in, because we may need it if we end up using #18826.

Nathann

comment:21 Changed 4 years ago by git

  • Commit changed from 769fc49bba2c1508cc61df75d91f4ea9c7214965 to b46fa1a5b3d462e89223e0cdf37d84c67d483e35

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

b46fa1aRestored ORIGINAL non-functional spkg-src as par Nathaann Cohen's request.

comment:22 follow-up: Changed 4 years ago by ncohen

Hey hey...

It's not exactly what "Could you sort this out (in the way that you prefer)" means.

Nathann

comment:23 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

This spkg-src does not produce the tarball in the ticket description.

comment:24 follow-up: Changed 4 years ago by ncohen

..............

What exactly are you trying to achieve, Jeroen ? Let this ticket go, he just wants to upgrade his package and his ticket does the job. His branch does not even touch the spkg-src file. Since when can a reviewer block a ticket like that?

Nathann

comment:25 Changed 4 years ago by ncohen

  • Status changed from needs_work to positive_review

comment:26 in reply to: ↑ 24 Changed 4 years ago by jdemeyer

Replying to ncohen:

His branch does not even touch the spkg-src file.

Right...

comment:27 in reply to: ↑ 22 ; follow-up: Changed 4 years ago by charpent

Replying to ncohen:

Hey hey...

It's not exactly what "Could you sort this out (in the way that you prefer)" means.

Sorry, Nathaan, sorry, Jeroen. I'm still running Homo Sapiens Sapiens version 1.0 : no telepathic interface. I do not understand nor guess what you two ghouls want.

Please feel free to do it yourself (if you can manage to agree on something...). I might review it.

Emmanuel Charpentier

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

It's not exactly what "Could you sort this out (in the way that you prefer)" means.

Sorry, Nathaan, sorry, Jeroen. I'm still running Homo Sapiens Sapiens version 1.0 : no telepathic interface. I do not understand nor guess what you two ghouls want.

My message was not clear: I was answering to your commit message. The commit is fine. Your commit message is "Restored ORIGINAL non-functional spkg-src as par Nathaann Cohen's request", which seemed a bit more abrupt than my previous "in the way that you prefer".

The branch is fine, I thank you for doing this last modifications, and I am sorry that we don't seem able in this community to sort this all out while preserving a light mood.

Your branch is okay, and does the job. We will probably rewrite the spkg-src script later anyway, if #18826 ends up being merged.

Nathann

comment:29 Changed 4 years ago by vbraun

  • Branch changed from u/charpent/upgrade_r_to_3_2_1 to b46fa1a5b3d462e89223e0cdf37d84c67d483e35
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.