Do not fetch VCS deps when reference didn't change

[maksbotan]

Pull Request Check List

If we have root -> foo -> VCSDependency(bar, ref="sha256"), and lock file contains bar @ sha256 and user ran poetry lock --no-update, do not fetch bar repo from git: as commit hash is explicitly locked, nothing could have changed.

• Added tests for changed code.
• Updated documentation for changed code.

As with #6130, I'll backport this to 1.1 once merged. And I'll need guidance on adding tests.

[radoering]

Although I think the change is functionally fine, I don't like it from an architectural point of view. I have thought about it for some time and finally created #6204. If this refactoring makes it to master, this PR can be adjusted accordingly.

[radoering]

Since #6204 has been merged you may rebase this PR. The required change should be much simpler now.

Re test: This change also fixes that the locked hash of a vcs dependency referencing a branch is updated when running poetry lock --no-update. I can provide a test for this later this week.

[maksbotan]

Thanks! I've followed #6204, it's great that it's merged.

I'll try to update this in a few days.

[maksbotan]

@radoering i've updated my branch to current master, incorporating your refactoring changes. Thanks!

[neersighted]

LGTM, but tests are needed. The existing tests for the provider should be good guidance @maksbotan -- but if you're still stuck on tests, @radoering are you still volunteering to get this one over the finish line?

[maksbotan]

I'd love to write a test, but I need an advice on what actually test for. Is there a way to check that provider does not actually call anything git-related (perhaps provider.get_package_from_vcs)? Maybe it can be done through mocker?

[radoering]

I added a test with a slightly different focus. It checks that a locked vcs dependency referencing a branch is not updated when running poetry lock --no-update.

@maksbotan You can probably add a similar test with a locked vcs dependency referencing a hash and check via a mock that get_package_from_vcs is not called.

@neersighted Do you really want to backport this? Afaik it's not a regression and it would require to backport #6204 or make significant changes in the backport...

[maksbotan]

Thanks!
What backport are talking about? I'm not sure I'm following

[radoering]

The label backport/1.2 has been set but I don't think we'll backport it.

[maksbotan]

Ah, I see, thanks. Without backport this will only hit 1.3, right?

[onerandomusername]

I'd like to see this backported if the 1.3 release is going to be anything like 1.2

[neersighted]

That's fair -- I had it in my head this was a regression, but the truth is that 1.1 exhibited this behavior as well.

[maksbotan]

@radoering I've added a new test, hope it's ok

[radoering]

LGTM 👍

tests were added

[maksbotan]

Thank you @radoering!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.