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

Track inputs for GeneratedAssetNodes #584

Merged
merged 4 commits into from
Nov 8, 2017
Merged

Track inputs for GeneratedAssetNodes #584

merged 4 commits into from
Nov 8, 2017

Conversation

jakemac53
Copy link
Contributor

This fixes a bug where the set of dependencies for a given generated asset could never be reduced except if those files were deleted.

It also partially unblocks #371.

@jakemac53 jakemac53 requested a review from natebosch November 7, 2017 23:05
@googlebot googlebot added the cla: yes Google is happy with the PR contributors label Nov 7, 2017
}, writer: writer);
});

test('invalidates generated assets based on graph age', () async {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that I deleted this test, it wasn't applicable any more and should have been deleted during the migration to content hashes

var removedInputs = node.inputs.difference(reader.assetsRead);
node.inputs.removeAll(removedInputs);
for (var input in removedInputs) {
// TODO: special type of dependency here? This means the primary input
Copy link
Member

Choose a reason for hiding this comment

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

IMO it's fine to always treat the primary input like it was read. The edge cases shouldn't break anything, at worst builds are slightly less optimal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya, thats pretty much my same line of thinking for now.

@jakemac53 jakemac53 merged commit 9bcc139 into master Nov 8, 2017
@kevmoo kevmoo deleted the track-inputs branch November 16, 2017 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google is happy with the PR contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants