Skip to content

Commit

Permalink
Merge pull request conda#9147 from msarahan/fix_neuter_history
Browse files Browse the repository at this point in the history
fix neutered specs not being recorded in history, leading to sadness
  • Loading branch information
jjhelmus authored Aug 29, 2019
2 parents 0fbd418 + d9befe3 commit b9a7c17
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 15 deletions.
1 change: 1 addition & 0 deletions conda/cli/main_remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def execute(args, parser):
link_precs=(),
remove_specs=(),
update_specs=(),
neutered_specs={},
)
txn = UnlinkLinkTransaction(stp)
handle_txn(txn, prefix, args, False, True)
Expand Down
10 changes: 7 additions & 3 deletions conda/core/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def match_specs_to_dists(packages_info_to_link, specs):
'link_precs',
'remove_specs',
'update_specs',
'neutered_specs'
))

PrefixActionGroup = namedtuple('PrefixActionGroup', (
Expand Down Expand Up @@ -208,7 +209,8 @@ def prepare(self):
for stp in itervalues(self.prefix_setups):
grps = self._prepare(self.transaction_context, stp.target_prefix,
stp.unlink_precs, stp.link_precs,
stp.remove_specs, stp.update_specs)
stp.remove_specs, stp.update_specs,
stp.neutered_specs)
self.prefix_action_groups[stp.target_prefix] = PrefixActionGroup(*grps)

self._prepared = True
Expand Down Expand Up @@ -261,7 +263,7 @@ def _get_pfe(self):

@classmethod
def _prepare(cls, transaction_context, target_prefix, unlink_precs, link_precs,
remove_specs, update_specs):
remove_specs, update_specs, neutered_specs):

# make sure prefix directory exists
if not isdir(target_prefix):
Expand Down Expand Up @@ -372,8 +374,10 @@ def _prepare(cls, transaction_context, target_prefix, unlink_precs, link_precs,

prefix_record_groups = [ActionGroup('record', None, record_axns, target_prefix)]

# We're post solve here. The update_specs are explicit requests. We need to neuter
# any historic spec that was neutered prior to the solve.
history_actions = UpdateHistoryAction.create_actions(
transaction_context, target_prefix, remove_specs, update_specs,
transaction_context, target_prefix, remove_specs, update_specs, neutered_specs
)
register_actions = RegisterEnvironmentLocationAction(transaction_context, target_prefix),
register_action_groups = [ActionGroup('register', None,
Expand Down
10 changes: 6 additions & 4 deletions conda/core/path_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -910,17 +910,19 @@ def reverse(self):
class UpdateHistoryAction(CreateInPrefixPathAction):

@classmethod
def create_actions(cls, transaction_context, target_prefix, remove_specs, update_specs):
def create_actions(cls, transaction_context, target_prefix, remove_specs, update_specs,
neutered_specs):
target_short_path = join('conda-meta', 'history')
return cls(transaction_context, target_prefix, target_short_path,
remove_specs, update_specs),
remove_specs, update_specs, neutered_specs),

def __init__(self, transaction_context, target_prefix, target_short_path, remove_specs,
update_specs):
update_specs, neutered_specs):
super(UpdateHistoryAction, self).__init__(transaction_context, None, None, None,
target_prefix, target_short_path)
self.remove_specs = remove_specs
self.update_specs = update_specs
self.neutered_specs = neutered_specs

self.hold_path = self.target_full_path + CONDA_TEMP_EXTENSION

Expand All @@ -932,7 +934,7 @@ def execute(self):

h = History(self.target_prefix)
h.update()
h.write_specs(self.remove_specs, self.update_specs)
h.write_specs(self.remove_specs, self.update_specs, self.neutered_specs)

def reverse(self):
if lexists(self.hold_path):
Expand Down
7 changes: 6 additions & 1 deletion conda/core/solve.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def __init__(self, prefix, channels, subdirs=(), specs_to_add=(), specs_to_remov
self.specs_to_add = frozenset(MatchSpec.merge(s for s in specs_to_add))
self.specs_to_add_names = frozenset(_.name for _ in self.specs_to_add)
self.specs_to_remove = frozenset(MatchSpec.merge(s for s in specs_to_remove))
self.neutered_specs = tuple()
self._command = command

assert all(s in context.known_subdirs for s in self.subdirs)
Expand Down Expand Up @@ -115,7 +116,7 @@ def solve_for_transaction(self, update_modifier=NULL, deps_modifier=NULL, prune=
force_remove, force_reinstall,
should_retry_solve)
stp = PrefixSetup(self.prefix, unlink_precs, link_precs,
self.specs_to_remove, self.specs_to_add)
self.specs_to_remove, self.specs_to_add, self.neutered_specs)
# TODO: Only explicitly requested remove and update specs are being included in
# History right now. Do we need to include other categories from the solve?

Expand Down Expand Up @@ -804,6 +805,10 @@ def _run_sat(self, ssc):
should_retry_solve=ssc.should_retry_solve
)

self.neutered_specs = tuple(v for k, v in ssc.specs_map.items() if
k in ssc.specs_from_history_map and
v.strictness < ssc.specs_from_history_map[k].strictness)

# add back inconsistent packages to solution
if ssc.add_back_map:
for name, (prec, spec) in iteritems(ssc.add_back_map):
Expand Down
12 changes: 10 additions & 2 deletions conda/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ def _parse_comment_line(cls, line):
item['update_specs'] = item['specs'] = specs
elif specs and action in ('remove', 'uninstall'):
item['remove_specs'] = item['specs'] = specs
elif specs and action in ('neutered', ):
item['neutered_specs'] = item['specs'] = specs

return item

Expand Down Expand Up @@ -277,6 +279,9 @@ def get_requested_specs_map(self):
spec_map.pop(spec.name, None)
update_specs = (MatchSpec(spec) for spec in request.get('update_specs', ()))
spec_map.update(((s.name, s) for s in update_specs))
# here is where the neutering takes effect, overriding past values
neutered_specs = (MatchSpec(spec) for spec in request.get('neutered_specs', ()))
spec_map.update(((s.name, s) for s in neutered_specs))

# Conda hasn't always been good about recording when specs have been removed from
# environments. If the package isn't installed in the current environment, then we
Expand Down Expand Up @@ -383,15 +388,18 @@ def write_changes(self, last_state, current_state):
for fn in sorted(current_state - last_state):
fo.write('+%s\n' % fn)

def write_specs(self, remove_specs=(), update_specs=()):
def write_specs(self, remove_specs=(), update_specs=(), neutered_specs=()):
remove_specs = [text_type(MatchSpec(s)) for s in remove_specs]
update_specs = [text_type(MatchSpec(s)) for s in update_specs]
if update_specs or remove_specs:
neutered_specs = [text_type(MatchSpec(s)) for s in neutered_specs]
if any((update_specs, remove_specs, neutered_specs)):
with codecs.open(self.path, mode='ab', encoding='utf-8') as fh:
if remove_specs:
fh.write("# remove specs: %s\n" % remove_specs)
if update_specs:
fh.write("# update specs: %s\n" % update_specs)
if neutered_specs:
fh.write("# neutered specs: %s\n" % neutered_specs)


if __name__ == '__main__':
Expand Down
2 changes: 1 addition & 1 deletion conda/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def explicit(specs, prefix, verbose=False, force_extract=True, index_args=None,
precs_to_remove.append(prec)

stp = PrefixSetup(prefix, precs_to_remove, tuple(sp[1] for sp in specs_pcrecs),
(), tuple(sp[0] for sp in specs_pcrecs))
(), tuple(sp[0] for sp in specs_pcrecs), ())

txn = UnlinkLinkTransaction(stp)
txn.execute()
Expand Down
4 changes: 2 additions & 2 deletions conda/plan.py
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ def revert_actions(prefix, revision=-1, index=None):

final_precs = IndexedSet(PrefixGraph(link_precs).graph) # toposort
unlink_precs, link_precs = diff_for_unlink_link_precs(prefix, final_precs)
stp = PrefixSetup(prefix, unlink_precs, link_precs, (), user_requested_specs)
stp = PrefixSetup(prefix, unlink_precs, link_precs, (), user_requested_specs, ())
txn = UnlinkLinkTransaction(stp)
return txn

Expand Down Expand Up @@ -395,7 +395,7 @@ def _inject_UNLINKLINKTRANSACTION(plan, index, prefix, axn, specs): # pragma: n
pfe = ProgressiveFetchExtract(link_precs)
pfe.prepare()

stp = PrefixSetup(prefix, unlink_precs, link_precs, (), specs)
stp = PrefixSetup(prefix, unlink_precs, link_precs, (), specs, ())
plan.insert(first_unlink_link_idx, (UNLINKLINKTRANSACTION, UnlinkLinkTransaction(stp)))
plan.insert(first_unlink_link_idx, (PROGRESSIVEFETCHEXTRACT, pfe))
elif axn in ('INSTALL', 'CREATE'):
Expand Down
2 changes: 0 additions & 2 deletions tests/core/test_solve.py
Original file line number Diff line number Diff line change
Expand Up @@ -2517,5 +2517,3 @@ def test_indirect_dep_optimized_by_version_over_package_count():
assert prec.build_number == 1
elif prec.name == '_dummy_anaconda_impl':
assert prec.version == "2.0"


13 changes: 13 additions & 0 deletions tests/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -2799,6 +2799,19 @@ def test_cross_channel_incompatibility(self):
'--dry-run', '-c', 'conda-forge', 'python',
'boost==1.70.0', 'boost-cpp==1.70.0', no_capture=True)

# https://github.com/conda/conda/issues/9124
@pytest.mark.skipif(context.subdir != 'linux-64', reason="lazy; package constraint here only valid on linux-64")
def test_neutering_of_historic_specs(self):
with make_temp_env('psutil=5.6.3=py37h7b6447c_0') as prefix:
stdout, stderr, _ = run_command(Commands.INSTALL, prefix, "python=3.6")
with open(os.path.join(prefix, 'conda-meta', 'history')) as f:
d = f.read()
assert re.search(r"neutered specs:.*'psutil==5.6.3'\]", d)
# this would be unsatisfiable if the neutered specs were not being factored in correctly.
# If this command runs successfully (does not raise), then all is well.
stdout, stderr, _ = run_command(Commands.INSTALL, prefix, "imagesize")


@pytest.mark.skipif(True, reason="get the rest of Solve API worked out first")
@pytest.mark.integration
class PrivateEnvIntegrationTests(TestCase):
Expand Down

0 comments on commit b9a7c17

Please sign in to comment.