repro: check for hash mismatch between deleted dependencies and upstream outputs.

[daavoo]

Only if --allow-missing is passed.

• Create tests/func/repro and extract pull and allow_missing to separate test files.

Closes #9530

[daavoo]

This is not pretty but I don't see any alternative

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (5c1487e) 90.55% compared to head (03869f4) 90.57%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9533      +/-   ##
==========================================
+ Coverage   90.55%   90.57%   +0.01%     
==========================================
  Files         470      472       +2     
  Lines       35939    36013      +74     
  Branches     5180     5182       +2     
==========================================
+ Hits        32545    32619      +74     
  Misses       2800     2800              
  Partials      594      594              

Impacted Files	Coverage Δ
tests/func/repro/test_repro.py	100.00% <ø> (ø)
dvc/repo/reproduce.py	95.49% <100.00%> (+0.04%)	⬆️
dvc/stage/__init__.py	91.66% <100.00%> (+0.03%)	⬆️
tests/func/repro/test_repro_allow_missing.py	100.00% <100.00%> (ø)
tests/func/repro/test_repro_pull.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dberenbaum]

This works, so not a blocker, but is there a way we can actually have dvc status --allow-missing return modified here like discussed in the issue?

It sounds like something I would expect to be possible, especially with the index changes from @efiop that I thought should make it easy to compare against virtual outputs.

[daavoo]

This works, so not a blocker, but is there a way we can actually have dvc status --allow-missing return modified here like discussed in the issue?

I think this P.R. should be merged and backported whereas the status change still needs discussion and would be a 3.0 change.

especially with the index changes from @efiop that I thought should make it easy to compare against virtual outputs.

I am not sure I follow how the index changes have anything to do with this, as this just involves checking upstream stages in the pipeline

[dberenbaum]

I think this P.R. should be merged and backported whereas the status change still needs discussion and would be a 3.0 change.

Okay, I'm fine to do it this way.

I am not sure I follow how the index changes have anything to do with this, as this just involves checking upstream stages in the pipeline

I wasn't that familiar with how status works and thought it would be possible. In theory, we could build a virtual index based on the outputs of the dvc.lock and the .dvc files and do a status check against that rather than against the workspace. From a quick look, it looks like it would basically do the same as this method except not against the workspace (which I don't see an easy way to do):

https://github.com/iterative/dvc/blob/3e0f2bafbbafe4c66cc74ff116692052b4d49a03/dvc/output.py#L590-L600

It would return a modified status when an upstream stage changed (since the hash of the out will change) without manually comparing upstream stages and changing the status inside repro.

[efiop]

Maybe we can assemble an index from deps and one from outs and then do an index diff?

[dberenbaum]

@daavoo Do you plan to merge as is? If so, can we get someone to take a look so we can unblock it please?

[daavoo]

@daavoo Do you plan to merge as is? If so, can we get someone to take a look so we can unblock it please?

I did not plan to implement a different solution for the release if this one works. cc @iterative/dvc could you review this?

[efiop]

@skshetry I see you've disabled auto-merge, was that intentional? Sorry, clicked and only then noticed.

[skshetry]

I was trying to merge, but noticed that some tests were failing. I have rerun them, waiting for them to be green. :)