Skip to content

Fixes for various hash issues#2626

Merged
tgamblin merged 7 commits intodevelopfrom
bugfix/various-hash-issues
Dec 19, 2016
Merged

Fixes for various hash issues#2626
tgamblin merged 7 commits intodevelopfrom
bugfix/various-hash-issues

Conversation

@tgamblin
Copy link
Copy Markdown
Member

@tgamblin tgamblin commented Dec 19, 2016 •
edited
Loading

Fixes #1992. Fixes #1178. Fixes #1377. Fixes #2123.

Fixes:

  • Behavior of satisfies() fixed for concrete specs with DAG hashes
    • spec.satisfies('/abc123') will only match another spec with same hash.
    • Prior behavior was just doing a match, which can be ambiguous.
  • Uninstall/activate/deactivate/whatever by hash is precise now.
  • installed_dependents() moved to DB from Package because we might not know a Spec's Package anymore. New version handles specs with unknown packages.
  • Minor: Fix default args in Spec.tree()
  • Minor: Fix exception name in spec.py

Improvements:

  • spack find and spack.store.db.query() are now faster for simple hash lookups
    • skips linear calls to satisfies() if query() only needs to look up by hash
    • spack find uses query() better.
  • More concise, readable output for disambiguating specs from the command line

Test:

  • Added a test to ensure satisfies() respects dag_hash() for concrete specs.

@davydden @eschnett @alalazo @adamjstewart @becker33

@tgamblin
Better output for disambiguate_specs()
0191ad2
@tgamblin
Fix wrong exception name.
6b6e420
@tgamblin
Fix default args in Spec.tree()
5eefa5e
@tgamblin
Fix satsifies(): concrete specs require matching by hash.
24ebe37 
- Fixes uninstall by hash and other places where we need to match a
  specific spec.

- Fix an error in provider_index (satisfies() call was backwards)

- Fix an error in satisfies_dependencies(): checks were too shallow.
@tgamblin
Move installed_dependents() to DB to avoid unknown package error.
0f74087
@tgamblin
Make spack find and sapck.store.db.query() faster for hashes.
c69411e
@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 19, 2016

@alalazo: This brings test time down from 8 min -> 4 min so I'm curious how this will do in conjunction with your pytest PR.

This was referenced Dec 19, 2016
Closed
Closed
Closed
Closed
@alalazo
Copy link
Copy Markdown
Member

alalazo commented Dec 19, 2016

This brings test time down from 8 min -> 4 min so I'm curious how this will do in conjunction with your pytest PR.

Do you mean on Travis? Because in the linked tests I still see 8-9 minutes. Anyhow, I see no improvement with respect to time on my laptop when rebasing the pytest PR, and apparently neither on Travis too 😭

@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 19, 2016

@alalazo I thought I meant Travis. But I must've looked at the wrong thing 😢

adamjstewart
adamjstewart reviewed Dec 19, 2016
View reviewed changes
lib/spack/spack/test/spec_semantics.py

# Simulate specs that were installed before and after a change to
# Spack's hashing algorithm. This just reverses s2's hash.
s2._hash = s1.dag_hash()[-1::-1]
Copy link
Copy Markdown
Member

@adamjstewart adamjstewart Dec 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, had no idea you could do this in Python. Very neat.

@adamjstewart
Copy link
Copy Markdown
Member

adamjstewart commented Dec 19, 2016

This is hard to test since the problems I was seeing are very intermittent (and may only occur after a hash change), but next time I can't uninstall something I'll give this a shot.

Btw, do activate/deactivate work with hashes yet? That was another thing that changing hashes caused headaches for me.

@tgamblin
Copy link
Copy Markdown
Member Author

tgamblin commented Dec 19, 2016

@adamjstewart: they should. Those were all the same bug -- that matching a concrete spec didn't require a hash match; the parser looked up the spec by hash and compared the whole DAG of the looked up spec. If one DAG was a superset of that, it would still match, so you could get multiple matches for one hash. This should fix the hash matching issues.

@tgamblin tgamblin merged commit a2b4146 into develop Dec 19, 2016
@tgamblin tgamblin deleted the bugfix/various-hash-issues branch December 31, 2016 04:58
@becker33 becker33 mentioned this pull request Jan 11, 2017
Closed
hartzell added a commit that referenced this pull request Mar 19, 2017
@hartzell
Fix for find --explicit #3374
49ed455 
This fixes the problem described in #3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in #2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
@hartzell hartzell mentioned this pull request Mar 19, 2017
Merged
tgamblin pushed a commit that referenced this pull request Mar 21, 2017
@hartzell @tgamblin
Fix for find --explicit #3374 (#3492)
9b5f5fc 
This fixes the problem described in #3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in #2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
diaena pushed a commit to diaena/spack that referenced this pull request May 26, 2017
@hartzell @diaena
Fix for find --explicit spack#3374 (spack#3492)
a95990d 
This fixes the problem described in spack#3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in spack#2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
xavierandrade pushed a commit to xavierandrade/spack that referenced this pull request Jun 16, 2017
@hartzell
Fix for find --explicit spack#3374 (spack#3492)
bde16f9 
This fixes the problem described in spack#3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in spack#2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
EmreAtes pushed a commit to EmreAtes/spack that referenced this pull request Jul 10, 2017
@hartzell @EmreAtes
Fix for find --explicit spack#3374 (spack#3492)
2dade8a 
This fixes the problem described in spack#3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in spack#2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
amklinv pushed a commit that referenced this pull request Jul 17, 2017
@hartzell
Fix for find --explicit #3374 (#3492)
fccf267 
This fixes the problem described in #3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in #2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
healther pushed a commit to electronicvisions/spack that referenced this pull request Jul 26, 2017
@hartzell @healther
Fix for find --explicit spack#3374 (spack#3492)
ea7f6a1 
This fixes the problem described in spack#3374, which describes `spack find` ignore explicit/implicit.

I believe that this was broken in spack#2626.

This restores the behavior of implicit/explicit for me.

I believe that it does not screw anything else up, but ....
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamjstewart adamjstewart adamjstewart left review comments

@alalazo alalazo alalazo approved these changes

@eschnett eschnett Awaiting requested review from eschnett

@davydden davydden Awaiting requested review from davydden

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@tgamblin @alalazo @adamjstewart