bugfix: uninstall should find concrete specs by DAG hash by tgamblin · Pull Request #13598 · spack/spack

tgamblin · 2019-11-05T09:43:47Z

This fixes a regression introduced in #10792. spack uninstall in an
environment would not match concrete query specs properly after the index
hash of enviroments changed.

Search by DAG hash for specs to remove instead of by build hash

This fixes a regression introduced in #10792. `spack uninstall` in an environment would not match concrete query specs properly after the index hash of enviroments changed. - [x] Search by DAG hash for specs to remove instead of by build hash

alalazo

I'm relatively new to this part of the code, but considering what I see here:

spack/lib/spack/spack/environment.py

Lines 1329 to 1347 in 527d639

    
           # If we are reading an older lockfile format (which uses dag hashes 
        
           # that exclude build deps), we use this to convert the old 
        
           # concretized_order to the full hashes (preserving the order) 
        
           old_hash_to_new = {} 
        
           self.specs_by_hash = {} 
        
           for _, spec in specs_by_hash.items(): 
        
               dag_hash = spec.dag_hash() 
        
               build_hash = spec.build_hash() 
        
               if dag_hash in root_hashes: 
        
                   old_hash_to_new[dag_hash] = build_hash 
        
               if (dag_hash in root_hashes or build_hash in root_hashes): 
        
                   self.specs_by_hash[build_hash] = spec 
        
           if old_hash_to_new: 
        
               # Replace any older hashes in concretized_order with hashes 
        
               # that include build deps 
        
               self.concretized_order = [ 
        
                   old_hash_to_new.get(h, h) for h in self.concretized_order]

the version in this PR seems more correct. However I have to say that I couldn't come up with a simple case in which spack uninstall <anything> has a behavior that is different on develop compared to this PR.

tgamblin added hashes environments labels Nov 5, 2019

tgamblin requested review from alalazo and scheibelp November 5, 2019 09:43

alalazo reviewed Nov 5, 2019

View reviewed changes

tgamblin merged commit a6ccb5b into develop Nov 5, 2019

tgamblin deleted the bugfix/env-uninstall branch December 24, 2019 09:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: uninstall should find concrete specs by DAG hash#13598

bugfix: uninstall should find concrete specs by DAG hash#13598
tgamblin merged 1 commit intodevelopfrom
bugfix/env-uninstall

tgamblin commented Nov 5, 2019

Uh oh!

alalazo left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	# If we are reading an older lockfile format (which uses dag hashes
	# that exclude build deps), we use this to convert the old
	# concretized_order to the full hashes (preserving the order)
	old_hash_to_new = {}
	self.specs_by_hash = {}
	for _, spec in specs_by_hash.items():
	dag_hash = spec.dag_hash()
	build_hash = spec.build_hash()
	if dag_hash in root_hashes:
	old_hash_to_new[dag_hash] = build_hash

	if (dag_hash in root_hashes or build_hash in root_hashes):
	self.specs_by_hash[build_hash] = spec

	if old_hash_to_new:
	# Replace any older hashes in concretized_order with hashes
	# that include build deps
	self.concretized_order = [
	old_hash_to_new.get(h, h) for h in self.concretized_order]

Conversation

tgamblin commented Nov 5, 2019

Uh oh!

alalazo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants