Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "Unexpected parent dependency" error #5729

Merged
merged 2 commits into from Jun 20, 2018
Merged

Fix "Unexpected parent dependency" error #5729

merged 2 commits into from Jun 20, 2018

Conversation

melix
Copy link
Contributor

@melix melix commented Jun 19, 2018

Context

It was possible for an edge to be reintegrated in the graph even if its
source node wasn't selected. This would result in an inconsistency when
writing, then reading the generated serialized graph. In particular,
when a node had a replacement, an that another edge in the graph caused
an upgrade of the source dependency, the older version of the source
node could have been seen as selected when it wasn't.

Fixes #4785

This PR is on master. If everything is fine, I'm going to backport it to release so that it can make it to 4.8.1, but timing is a bit short.

@melix melix added this to the 4.8.1 milestone Jun 19, 2018
@melix melix self-assigned this Jun 19, 2018
@melix melix requested a review from ljacomet June 19, 2018 09:53
@@ -137,8 +139,10 @@ public void failWith(Throwable err) {
}

public void restart() {
removeFromTargetConfigurations();
attachToTargetConfigurations();
if (from.isSelected()) {
Copy link
Member

@ljacomet ljacomet Jun 19, 2018

Choose a reason for hiding this comment

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

If I read this PR correctly for production code, this change is the fix. All the other changes are style or performance related. If that's the case, is it possible to isolate the fix in its own commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Splitted into 2 commits.

Copy link
Member

Choose a reason for hiding this comment

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

Great

In particular, avoid the creation of an iterator when lists are empty.
It was possible for an edge to be reintegrated in the graph even if its
source node wasn't selected. This would result in an inconsistency when
writing, then reading the generated serialized graph. In particular,
when a node had a replacement, an that another edge in the graph caused
an upgrade of the source dependency, the older version of the source
node could have been seen as selected when it wasn't.

Fixes #4785
@melix melix merged commit abdd883 into master Jun 20, 2018
@melix melix deleted the cc/dm/issue-4785 branch June 20, 2018 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants