Feature Request: remove need for multiple spago.dhall config files by defining output targets in spago.dhall
#681
Comments
|
I spent some time looking at this issue again. I don't think a target should have a
Specifying different
|
|
Thanks for taking a look at this @JordanMartinez! I have a few thoughts on this (that I hope won't shuffle much the implementation work you already started) 😊 I originally envisioned this to be part of the config format redesign that will happen once the Registry ships (as I describe better in #766 (comment)).
This is exactly the usecase that I had in mind with the
So the idea behind this was to make this process smoother.
I am not sure what's the rationale behind using the
I'd like to use Similarly, I think we should use
I wouldn't like us to migrate user files until we do the Final Migration Once The Registry Ships:tm:, as I feel that's going to add so much code in order to support multiple migrations, etc. Lines 362 to 371 in 7728ce4
This is fair, but something that we should definitely avoid would be to implement this in a way that we'd have to break later, once we want to support publishing of multiple packages from a single config. Other than the above concerns, this looks good. Implementation-wise it might be troublesome to patch the part that manipulates the raw Dhall AST, that is called in |
|
I decided to try tackling this while on vacation when I had a few spare moments. I didn't have access to the Internet all the time, so it made doing some of this harder. So, that's why I didn't respond immediately to your comment above. Was your above comment meant to be added to my PR or to this issue? Also, should I wait until the Registry is out before continue work on this? I get the concern for migrating
Originally, I was going to use
Yeah, I've implemented that in my PR. Both
That's been done, too. I update the dependencies based using this resolution method:
This resolution process covers all of these possibilities: -- literal target with no dependencies on earlier targets
{ targets =
{ main =
{ dependencies = ["foo"]
, sources = ["src/**/*.purs"]
}
}
}
-- literal target with a dependency on earlier target
let dep =
{ dependencies = ["foo"]
, sources = ["src/**/*.purs"]
}
in
{ targets =
{ main =
{ dependencies = dep.dependencies # ["foo"]
, sources = dep.sources # ["src/**/*.purs"]
}
}
}
-- bound target with literal target
let dep =
{ dependencies = ["foo"]
, sources = ["src/**/*.purs"]
}
in
{ targets =
{ main = { dep }
}
}
-- bound target with dependency on earlier target
let main =
{ dependencies = ["foo"]
, sources = ["src/**/*.purs"]
}
let test =
{ dependencies = main.dependencies # ["spec"]
, sources = main.sources # ["testc/**/*.purs"]
}
in
{ targets =
{ main
, test
}
} |
|
Where those different "targets" are going to be build to (to what output directory) by the compiler? |
|
For now, I've assumed that all targets' compiled output is stored in the same |
|
Also, one issue I've come across while working on this. If we have multiple targets, how would that work with getting autocompletion, etc. via the IDE? For example, let's say I have 4 targets. Since the IDE expects there to be one command that it runs to compile the code, I wouldn't want to run If one is using multiple targets, they likely have some sort of dependency chain between the targets, such that one target depends on all others and using that target would for Moreover, having a dependency chain between targets is not always guaranteed. In a monorepo setup, there might be multiple directories with each doing one thing separate from the others. In that case, one would need to combine all the targets into one by defining a fifth target. We could call this fifth target let all =
{ dependencies = target1.dependencies # target2.dependencies # ... # target4.dependencies
, sources = target1.sources # target2.sources # ... # target4.sources
}
in
{ ...
, targets = { ..., all }
}This However, I don't like this workaround. Every time I add a new target, I would need to update
Using the above example, Spago could support a virtual target called "all" that combines the dependencies and sources of all other targets within the Another idea related to this one. I hate having to "install" let main = { dependencies = [ "prelude" ], sources = [ "src/**/*.purs" ] }
let main-repl = main.with dependencies = main.dependencies # [ "psci-support" ]However, now we have a new problem. If I want to get REPL access to my other targets, I need to add a The difference between the |
|
I generally like the idea of multiple Trying to be too specific about what those What I think how this feature should be generally designed:
As for the development/IDE process in a default assumed workflow the "default" target should be used, and the default target should really accumulate all the (needed) dependencies and sources from other targets, but it should be stated in config explicitly. For example, I do it now this way for my monorepo setup and have no problems with it except the one issue I created recently. So I just have dhall config for each package with its deps (which will be published anyway in its own git repos to be consumed as the current packaging infrastructure assumes) and the root config that is used for development, which happens to include all the deps from packages and additional common deps for the project (e.g, deps for running tests): let package1 = ./packages/package1/spago.dhall
let package2 = ./packages/package2/spago.dhall
in { name = "monorepo"
, dependencies =
[ "spec"
, "psci-support"
]
# package1.dependencies
# package2.dependencies
, packages = ./packages.dhall
, sources =
[ "packages/**/src/**/*.purs"
, "packages/**/test/**/*.purs"
]
}With |
|
Below is my understanding of your comment above. Correct me where my understanding is wrong:
If I'm understanding you correctly, this sounds like what I will call a "target-then-root lookup resolution." If { dependencies = [ "foo" ]
, sources = [ "src/**/*.purs" ]
, output = "./output"
, targets =
{ main = { }
, test = { sources = [ "test/**/*.purs" ] }
, foo = { output = "./barOutput", sources = [ "bar/**/*.purs" ] }
}
}
Because the { dependencies = [ "foo" ]
, sources = [ "src/**/*.purs" ]
, output = "./output"
, {- other fields -}
}
This is basically your solution to the "which target does Spago use when one isn't specified" problem.
This is your solution to the "which target does an IDE use so that all desired code is compiled" problem. Based on my understanding above, I'll now be responding to your comment in general, and then to specific points. If my understanding is wrong, then my arguments will be pointless. I dislike most of this idea for one reason: it would make Spago do work that should be done via Dhall. I don't think Spago should need to do anything more than just lookup a key and use it. For example, rather than having Spago figure out what some target's I also dislike this idea because it's not clear what to do when you run The "target-then-root lookup resolution" also reminds me of
I disagree with this because I don't see why multiple targets should be optional. First, a "single target" format is isomorphic to a "multi-target format" that has a single target. If we go with #811, the "single target" format below is the same as the "multi-target" format that follows. {- single-target (current Spago format) -}
{ dependencies = [ "foo" ]
, sources = [ "src/**/*.purs" ]
}
{- multi-targets with one literal target -}
{ targets =
{ main =
{ dependencies = [ "foo" ]
, sources = [ "src/**/*.purs" ]
}
}
}Second, if migrating is a concern, "single target" formats can be migrated to "multi-target" formats easily. In fact, what we have now (v2 config) was previously migrated from a different format (v1 config). The "multi-targets" format would simply be a v3 config. Third, people using Spago almost always use
I don't think your proposed solution solves the "which target does Spago use by default" problems completely. All spago commands (e.g. So, in your proposed idea, what would happen if you run
This doesn't seem to solve the problem completely. Is there ever a time where you'd want the "default target" to refer to one target for
I think this is a valid point and the only part in your idea that I agree on 😄. I've assumed that all targets should share the same output directory, but there might be some targets that have the same module name because their code should never be run together. For example, let's say we have two targets, |
|
First, I don't think that the points I laid out above go in contradiction with discussed proposal here. It is just my vision of the feature that can be wrong in some way and can be corrected, so I just wanted to stress some points that just should be considered more carefully.
I've accented the word possibly in that point because it should be considered if it really the correct approach to extend target config with the values from the root, or to force the explicit configuration of the targets without involving any kind of inheritance.
I propose to come from the user perspective first of all, not from the dhall features standpoint. If it is reasonable from the user standpoint to inherit some values from the root then I believe it should be considered. Though I'm not sure that any values e.g,
There are some questions that should be answered:
So when those questions are resolved then it becomes clear what target runs "by default" (when no target is specified). I didn't like this thing called Also, there may be idea of combined targets specified in the config like list of other defined targets: targets =
{ test = { ... }
, rest = { ... }
...
, combined = [ "test", "rest" ]
}I just don't know if
As I stated it is assumed as the "default approach", if you need something else you can configure IDE tooling to run differently than the default build command.
If my
If this convention is considered to be useful, it should exist as probably a shortcut for |
|
Thank you for the great discussion here! I'll try to summarize here my wishes for how this should look like:
|
I still didn't get what nice dhall feature I was neglecting in my points. It sounds it is about some complex operations ("work") that could be avoided, but I just was proposing to inherit some values from the config root if it is appropriate and make sense (sematically) from the usage standpoint. If inheritance from the root is no good because it adds implicitity, it ok that everything should be explicitly encoded in config for all targets. Thought it may bring other problems, e.g, if user forgets to put some optional key in target config, say "output", it would be nice just have this property in the root and share its value between the targets, not to force all the targets to include that key.
I wouldn't like to oppose here much because it makes no big difference, but to me this seems to be a bit inconsistent. To me reserving a symbolic name that assumes a special kind of operation messes things up. A special flag reflects the special purpose and underlying operations. If there are supposed to exist other "virtual targets" (besides "all") a special flag like "--virtual-target" would be justified, at least it doesn't mess things up. And I think that from the user perspective a simple flag-like e.g., just
Not sure why should it be required if for example a target is not supposed to be run, for example, just to be built? I believe there are may exist targets that are not a subject for a run at all, or maybe the user won't be using |
This sounds like it should be its own issue so it can be implemented separately from this one. |
Could you explain your reasoning behind this? If each target defines a single entry point, then would the |
|
Just wanted to summarize things so far. There are a few problems we're trying to solve along with their solutions:
Problem 5's current format looks like the following. The let packages = ./packages.dhall
{- `main` = what `spago.dhall` in the Single Target Format would refer to. -}
let main = { dependencies = [ "dep1" ,"dep2" ]
, sources = [ "src/**/*.purs" ]
, output = "./someOtherDir" {- Note: this key is optional -}
}
{- `test` = what `test.dhall` in the Single Target Format typically refers to. -}
let test = { dependencies = main.dependencies # [ "spec" ]
, sources = main.sources # [ "test/**/*.purs" ]
}
{-
let otherTarget = { dependencies = main.dependencies
, sources = main.sources # [ "examples/**/*.purs" ]
}
-}
in
{ name = "my-project"
, packges
, targets = { main, test {-, otherTarget -} }
}Implementing the above code is straight forward except for Problem 4. If we get that wrong, then this effort will fail. Also, if we add the optional |
|
I checked out a separate branch that was the same as the In other words, I think the work that remains before targets can be implemented are merging the below PRs in the following order (and fixing merge conflicts as they arise):
|
|
As explained in #811, I do the following to ensure installing a package works:
This new algorithm would cause issues if run with the |
|
This is now implemented, see the new docs on monorepo support |
In an attempt to solve #416, I described in this comment the idea of "output targets" (since
targetalready refers to the output file one produces when bundling an app).Problem
If you want to add "developer dependencies," "test dependencies," or do a mono-repo, you have to define one
spago.dhallconfig file for each output target. Hence, one may find aspago.dhallfile, atest.dhallfile, anexamples.dhallfile, and sometimes abenchmark.dhallfile.Rather than duplicating our work, this feature would define all of those targets within a single
spago.dhallfile.Below is an example of what
spago.dhallwould look like after this change:Unanswered Questions
For 3, we could "desugar" commands:
spago build->spago --output main buildspago run->spago -o main runspago test->spago -o test runI don't think this feature should remove the
-x config.dhallpart. Both features could exist at the same time.The text was updated successfully, but these errors were encountered: