Skip to content

Allow spack create to handle packages with period in name#2351

Merged
tgamblin merged 2 commits intospack:developfrom
adamjstewart:fixes/period-in-url
Dec 5, 2016
Merged

Allow spack create to handle packages with period in name#2351
tgamblin merged 2 commits intospack:developfrom
adamjstewart:fixes/period-in-url

Conversation

@adamjstewart
Copy link
Copy Markdown
Member

Fixes #2346.

Some package URLs include a period in their name, such as:

https://pypi.io/packages/source/b/backports.ssl_match_hostname/backports.ssl_match_hostname-3.5.0.1.tar.gz

When running spack create, this was being interpreted as a package name of ssl_match_hostname and a NAMESPACE of backports. This PR replaces periods and underscores with dashes, so as to match the rest of Spack. I've never used Spack's NAMESPACE support, so can someone who does test this PR for me?

If anyone wants me to, I can rename packages like the_silver_searcher and SAMRAI to be lowercase and separated by dashes. And then we can get into the debate over R vs. r 😄

@adamjstewart
Copy link
Copy Markdown
Member Author

adamjstewart commented Nov 16, 2016

@alalazo @tgamblin Another random Travis failure. Seems to be getting more frequent lately.
https://travis-ci.org/LLNL/spack/jobs/176425363

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Nov 16, 2016

@adamjstewart Is there an issue open on those random failures ? If not can you open it and link at least the PRs where it happened ? I think it would be good to see if the causes are always the same. In this case it seems something related to the fetchers.

@adamjstewart
Copy link
Copy Markdown
Member Author

@alalazo How do I look up the URL for the Travis build that failed?

@alalazo
Copy link
Copy Markdown
Member

alalazo commented Nov 16, 2016

If people didn't force push, I guess you can just click on the X sign next to the failing commit. If people did force push you have to check build-history in LLNL/Travis manually (or at least that's what I do...).

@adamjstewart
Copy link
Copy Markdown
Member Author

It looks like even if you close and reopen, it overwrites the X with a new check.

@adamjstewart
Copy link
Copy Markdown
Member Author

Ping @tgamblin @becker33

@adamjstewart
Copy link
Copy Markdown
Member Author

Ping @tgamblin

@becker33
Copy link
Copy Markdown
Member

becker33 commented Dec 2, 2016

This looks good to me, but @tgamblin is more familiar with the namespace feature so I'll probably leave it until he gets a chance to look at it.

@adamjstewart
Copy link
Copy Markdown
Member Author

Ping @tgamblin. This will help keep things uniform, in line with #2475.

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Dec 5, 2016

@becker33: this isn't actually about namespaces, just packages that have . in the URL.

@adamjstewart: This looks good, I'm just curious whether you think it would be better to handle the conversion of _ and . to - outside of parse_name_offset, in case we ever want to recover the original name from the tarball. Seems like it would be simpler to keep the existing parse_name_offset functionality and wrap the choices we've made w.r.t. package name formats up into a sanitize_package_name function, instead of putting them together.

@adamjstewart
Copy link
Copy Markdown
Member Author

@tgamblin I'll leave it up to you. I'm not sure what the original tarball name would be useful for, but you're welcome to push any changes to my branch. My only request is that my changes to the url_parse tests remain in effect. That would mean any calls to url.parse_name_and_version should return the sanitized name.

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Dec 5, 2016

Ok I thought about this more. I actually like the consistency and don't feel like adding a special case we're not likely to use.

@tgamblin tgamblin merged commit 161d620 into spack:develop Dec 5, 2016
@adamjstewart adamjstewart deleted the fixes/period-in-url branch December 5, 2016 19:10
kserradell pushed a commit to kserradell/spack that referenced this pull request Dec 9, 2016
* Allow spack create to handle packages with period in name

* Update tests to handle new package name detection scheme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants