Skip to content

Use zlib's "fossil" site for old tarballs#2735

Merged
tgamblin merged 2 commits intospack:developfrom
hartzell:bugfix/use-zlib-fossil-site
Jan 4, 2017
Merged

Use zlib's "fossil" site for old tarballs#2735
tgamblin merged 2 commits intospack:developfrom
hartzell:bugfix/use-zlib-fossil-site

Conversation

@hartzell
Copy link
Copy Markdown
Contributor

@hartzell hartzell commented Jan 4, 2017

Following citibeth's suggestion in #2732, use zlib's "fossil"
site (not to be confused with the sqlite team's VCS...) for retrieving
old tarballs.

Digests for 1.2.{8,10} match and both install for me on CentOS 7.

This is an alternative to #2733, which relies on sourceforge (something I try to avoid...).

Use zlib's "fossil" site for old tarballs
02461bc 
Following citibeth's suggestion in spack#2732, use zlib's "fossil"
site (not to be confused with the sqlite team's VCS...) for retrieving
old tarballs.

Digests for 1.2.{8,10} match and both install for me on CentOS 7.
@hartzell hartzell mentioned this pull request Jan 4, 2017
Closed
@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented Jan 4, 2017

@citibeth -- just in case you haven't already been deluged with email etc... about this....

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 4, 2017

Thanks for taking this up. I'm not able to do this today, unfortunately. But I recommend the following. Basically, the fossils URL should be the default for the package. The latest version of Zlib will download specially from the non-fossils directory.

class Zlib(AutotoolsPackage):
    """A free, general-purpose, legally unencumbered lossless
       data-compression library."""

    homepage = "http://zlib.net"
    url = "http://zlib.net/fossils/zlib-1.2.8.tar.gz"

    version('1.2.10', 'd9794246f853d15ce0fcbf79b9a3cf13',
        url='http://zlib.net/zlib-1.2.10.tar.gz')
    # author had this to say about 1.2.9....
    # (Can a URL be provided here for more info?)
    # Due to the bug fixes, any installations of 1.2.9 should be immediately replaced with 1.2.10.
    version('1.2.8', '44d667c142d7cda120332623eab69f40')

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 4, 2017

Also.. can someone please point me to what list_url is for?

@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Jan 4, 2017

The default URL can probably be the new one and the list_url can be the old one. If we substitute the version with url and it exists, we use it. If it doesn't exist, we try searching list_url as a backup. So the list_url should be http://zlib.net/fossils/. Btw, this doesn't fix the real underlying issue I brought up in #2725.

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 4, 2017 via email

Use zlib's "fossil" URL as the one true URL
0b26ad6 
Everything seems to be available at zlib's "fossil" URL, so just use
it as the one and only url.

(and fix a flake8 complaint about a comment)
@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented Jan 4, 2017

@adamjstewart -- how does this look to you?

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Jan 4, 2017

@citibeth: did you try searching for list_url in the docs? It's in the packaging guide.

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 4, 2017 via email

@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Jan 4, 2017

This looks good. At first I didn't see 1.2.10 on that list because it was sorted alphabetically, not numerically. So no list_url necessary.

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Jan 4, 2017

@citibeth: that is a good point. We should update the docs to mention that the list_url can be an alternate download location. Although if you think about it, the fact that you can use it to find tarballs kind of implies that...

adamjstewart referenced this pull request Jan 4, 2017
@BarrySmith @tgamblin
Update to latest zlib version, server no longer provides older version (
2322ded 
#2720)

* Update to latest zlib version, server no longer provides older version

Funded-by: IDEAS
Project: IDEAS/xSDK

* Add alternative URL for previous release of zlib
@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 4, 2017

Although if you think about it, the fact that you can use it to find tarballs kind of implies that...

Even after thinking about it, that is not clear or obvious to me.

We should update the docs to mention that the list_url can be an alternate download location.

Sorry, this feels kludgy; like it's the first thing that came to somebody's mind, without any design review. A more obvious (to me) way to provide alternative download locations would be something like:

url = ('http://location1.org/mypackage-1.1.1.tar.gz', 'http://location2.net/folder/mypackage-1_1_1.tar.gz')

This has many advantages:

  1. More intuitive, less kludgy
  2. Avoids repurposing list_url for something it's not
  3. Allows >2 URLs in the list of alternatives
  4. Obvious extensions allow list_url to be a list/tuple as well.

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented Jan 4, 2017

As it currently stands, the PR no longer uses list_url, though the discussion seems interesting.

@adamjstewart seems happy with it in its new form.

LGTM (let's see if that confuses @tgamblin 😄 [joke from another PR...])

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Jan 4, 2017

@citibeth: sure, that's potentially better. That's not what's implemented, and this PR is about fixing zlib. I am not sure how complicated things become if we consider your suggestion along with the mirror system, and we're actually planning to set up a mirror for all the packages, so I am not sure how useful that feature will really be.

@tgamblin tgamblin merged commit 4e65325 into spack:develop Jan 4, 2017
@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Jan 4, 2017

@hartzell: did I get it right?

@hartzell
Copy link
Copy Markdown
Contributor Author

hartzell commented Jan 4, 2017

@tgamblin: exactly! ;)

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 5, 2017

@hartzell Thanks!

@citibeth
Copy link
Copy Markdown
Member

citibeth commented Jan 5, 2017

we're actually planning to set up a mirror for all the packages

I like that idea!

@hartzell hartzell deleted the bugfix/use-zlib-fossil-site branch January 5, 2017 17:37
@citibeth citibeth mentioned this pull request Jan 5, 2017
Merged
@alalazo alalazo mentioned this pull request Jan 10, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin tgamblin approved these changes

@adamjstewart adamjstewart adamjstewart approved these changes

Assignees

No one assigned

Labels

new-package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hartzell @citibeth @adamjstewart @tgamblin