Add output folder sharing and --no-share-output flag to build
#422
Conversation
| (length (System.FilePath.splitSearchPath path)) | ||
| (length <$> current) | ||
| then Just path | ||
| else current |
There was a problem hiding this comment.
This looks good! However I wonder how brittle it is in the end.
Just for inspiration I went looking what Dhall itself actually does with the graph we use here, and turns out it's being used to print dot graphs.
Here's the implementation, and here's a result for our monorepo:
So it looks like we have all the info available to find out which one is the "rootmost" import for all the cases (while here we'd get it wrong if the "main packages.dhall is not the highest in the file hierarchy). So looking at the graph (which we can build because we get all the parent-child relationships in the graph) a better heuristic than the one I proposed before seems to be "pick the topmost packages.dhall which is not a remote import, an import as Location, or as Text"
There was a problem hiding this comment.
Ahh right - sounds like we have everything we need to do this nicely. Will give this a smash.
There was a problem hiding this comment.
Does this implementation work?
There was a problem hiding this comment.
I would have liked something more like "is this file importing this other one" rather than "is this directory shorter", but let's go with this for now
|
Hello! I think everything above should be resolved now - added another test to be sure that including a module from higher up in the tree than |
danieljharvey
changed the title
|
Actually - looks like this goes wrong when running tests - it builds everything then the |
|
@danieljharvey is it because |
|
Yeah - looks like it - going to make a more generic “getOutputPath” function to use here - how do you feel about a |
|
@danieljharvey yeah |
WIP Remove garbage Remove yarn.lock Remove package.json Fixed
|
Hey @f-f - this is working for me now, and have addressed changes from above. |
f-f
left a comment
f-f
left a comment
There was a problem hiding this comment.
@danieljharvey looks great, thanks for doing this! 💯
| (length (System.FilePath.splitSearchPath path)) | ||
| (length <$> current) | ||
| then Just path | ||
| else current |
There was a problem hiding this comment.
I would have liked something more like "is this file importing this other one" rather than "is this directory shorter", but let's go with this for now
f-f
changed the title
--no-share-output flag to build
This PR disables output sharing that was introduced in #422. The reasoning for this is twofold: 1. this change seems to break `psc-ide` and various other tooling 2. Spago messing with people's `output` folder doesn't really feel right, because e.g. it forces people to depend on wherever Spago decides the output folder to be in their project We could of course just invert the logic of the flag and make it opt-in instead, but I just don't think we should encourage point (2) above. The "proper" way to do this IMHO is that Spago could do some kind of "build artifact caching": i.e. sharing compiled JS between projects compiled with the same set of packages and compiler. This is a strictly better solution, because: - it doesn't mess with people's output folder - it enables artifact sharing even between different projects and not just in the same monorepo
This commit disables output sharing that was introduced in #422. The reasoning for this is twofold: 1. this change seems to break `psc-ide` and various other tooling 2. Spago messing with people's `output` folder doesn't really feel right, because e.g. it forces people to depend on wherever Spago decides the output folder to be in their project We could of course just invert the logic of the flag and make it opt-in instead, but I just don't think we should encourage point (2) above. The "proper" way to do this IMHO is that Spago could do some kind of "build artifact caching": i.e. sharing compiled JS between projects compiled with the same set of packages and compiler. This is a strictly better solution, because: - it doesn't mess with people's output folder - it enables artifact sharing even between different projects and not just in the same monorepo
3 participants
Description of the change
As per issue #377 building in a Monorepo ends creating a lot of duplication. This change adds a build flag
--sharedOutputwhich will askpursto output into the "root" folder (hopefully where the mainpackages.dhalllives, but basically the highest up source file in the tree that Dhall tells us about).Checklist:
READMEP.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊