-
Notifications
You must be signed in to change notification settings - Fork 121
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
Support importing of Makefiles (as opposed to extending) #322
Comments
thanks for the feedback. i'm wondering what cargo-make provides now that can solve this and what needs development to help out making it simpler. to solve that, you have the possibility to define the current working directory on the task level and use cargo-make provided env vars and do relative paths from there. [tasks.mybootloader-build]
cwd = "${CARGO_MAKE_WORKING_DIRECTORY}/somedir"
# or in case of a workspace you might want to use the workspace root as the root path:
# cwd = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/somedir"
command = "..." I'm not entirely familiar with your use case so i'm wondering if this solves the issue for you? |
This works from the master makefile but it's a sub optimal solution as this breaks when using the nested makefiles directly. Eventhough there probably is a functional solution in this case, having to "work around" the limitations isn't very intuitive or enjoyable. I believe my approach as outlined above adds a necessary primitive that has many more uses than my specific problems. If you think about it, extending and importing are two conceptually distinct operations that both have their use cases. |
I just want to understand the problem better before i change 30 files and we end up with same issue. Therefore i'm trying to see what the best solution for your issue is, before jumping on the first one. So there are options today to resolve that, and they might be simple enough. option 1 - define new env var in makefiles and use in common as cwdSo you could have in workspace level and module level makefiles, in the env block a definition of that common working directory, [env]
COMMON_WORKING_DIRECTORY="../../mydir" naturally the value will be based on the module/workspace makefile you are editing. [tasks.mybootloader-build]
cwd = "${COMMON_WORKING_DIRECTORY}"
# optionally you can define a condition to ensure that the env is set
condition = { env_set = [ "COMMON_WORKING_DIRECTORY" ] }
command = "..." option 2 - wrap common task with local task with cwdyou can do the same trick by having some local task change the cwd and then call the common task with run_task. # module specific makefile
[tasks.mybootloader-build-wrapper]
cwd = "../../common"
run_task = "mybootloader-build" and in common makefile you don't have to do anything at all. I'm sure you won't have to do this for every task, only for the specific problematic task that has relative paths messed up. |
Thanks for the answer! That makefiles store metadata about where they are defined is pretty much the core of my idea. Instead of loading the different tomls and putting it all into one datastructure an imported makefile is it's own datastructure (which again could import or extend other makefiles). The only thing that connects them in the code is a reference from the importer to the importee. What this solves is the problem of nested projects which I think is a fairly common use case. |
What if we store the file and directory that originally defined the task (ignoring the fact that it might be updated by extending files), then allowing you to do something like this: [tasks.sometask]
cwd = { type = 'makefile' } which you can set in specific tasks that need it in your common makefile. i'm against importing as it brings in a lot of complexity and confusion. |
I agree that import functionality would be kind complex to implement. Your idea seems like a good compromise. Having the cwd type as makefile (i.e. use the cwd of the declaring makefile) should be the default in my opinion (just like declaring an env var uses the cwd of the declaring makefile) but I guess that would break backwards compatibility |
…EFILE, CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY environment variables #322
@coffeenotfound just pushed a commit to the 0.24.1 development branch.
This means, that in your common makefile, you can do the following: [tasks.sometask]
cwd = "${CARGO_MAKE_CURRENT_TASK_INITIAL_MAKEFILE_DIRECTORY}" Please try it out and tell me if it solves your issue. |
This is now officially release so i'm closing this issue. |
This does solve my issue! Thanks for the quick implementation! |
Features Description
This feature allows you to "import" another Makefile and depend on its tasks without extending it.
Describe the solution you'd like
Importing a makefile as opposed to extending it would allow depending on its tasks while still treating it as a seperate makefile. This is important for relative paths which break upon extending from a master makefile.
Makefiles would be imported under a given name in the imports table and it's (exported) tasks are then prefixed accordingly with the chosen name. (See example below)
Tasks should still be deduplicated across imported makefiles. E.g. in a diamond pattern of makefiles (A -> B -> D and A -> C -> D), a task in makefile D is only invoked once even if a task of B and one of C depend on it.
Tasks should also have a new boolean key "exported" (true by default) similar to "private" which controls whether the task is visible to importing makefiles.
Code Sample
The text was updated successfully, but these errors were encountered: