snowleopard / build

Build Systems à la Carte
MIT License
247 stars 18 forks source link

Multi handles inputs incorrectly #7

Closed snowleopard closed 6 years ago

snowleopard commented 6 years ago

@rotsor spotted today that the implementation of multi always returns Just in the second case:

https://github.com/snowleopard/build/blob/master/src/Build/Multi.hs#L20

But what if we ask the resulting Tasks to build a list that contains input keys? We'll fetch them! This will lead to a cyclic dependency, breaking the build system.

The fix doesn't seem to be difficult: presumably, we need to apply tasks to all keys and if any of them returns Nothing we should also return Nothing. It's a bit awkward, but I don't see a better fix.

@ndmitchell Thoughts?

P.S.: Actually the correct check should probably be isInput = any (isNothing . tasks . partition) keys.

Rotsor commented 6 years ago

Thinking a bit more about this, maybe there's no cyclic dependency after all: [x;y] and [x] are distinct keys after all, so it seems ok for the former to depend on the latter.

On Tue, May 8, 2018, 00:02 Andrey Mokhov notifications@github.com wrote:

@Rotsor https://github.com/Rotsor spotted today that the implementation of multi always returns Just in the second case:

https://github.com/snowleopard/build/blob/master/src/Build/Multi.hs#L20

But what if we ask the resulting Tasks to build a list, which contains input keys? We'll fetch them! This will lead to a cyclic dependency, breaking the build system.

The fix doesn't seem to be difficult: presumably, we need to apply tasks to all keys and if any of them returns Nothing we should also return Nothing. It's a bit awkward, but I don't see a better fix.

@ndmitchell https://github.com/ndmitchell Thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snowleopard/build/issues/7, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUa7AQ37HgJdVXPbHvdRuYe8ucvw7Vxks5twMRvgaJpZM4T1tu7 .

snowleopard commented 6 years ago

Oops. Indeed, looks fine in the morning :-)