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

Support for including only parts of the default makefile without risk of naming collisions #201

Closed
ssokolow opened this issue Mar 12, 2019 · 21 comments
Assignees
Milestone

Comments

@ssokolow
Copy link

ssokolow commented Mar 12, 2019

Features Description
I'm working on modernizing my rust-cli-boilerplate and I care very much about providing a clean, coherent development experience... but the default makefile tasks follow a philosophy diametrically opposed to how the justfile I'm interested in replacing is set up.

(Most notably, I consider it borderline insane to make the intutive and concise build be a plumbing command (To use the git terminology) which doesn't execute a pre- and post- command, while the stuff which seems to be what an "experienced cargo-make user" should be using habitually, like the *-flow tasks, is verbose in a way I wouldn't expect from a porcelain command. )

I also find that having all those default tasks I never use and never test for compatibility with my codebase makes --list-all-steps useless when it should be a more convenient alternative to pulling up my project template's README again. (Heck. Ideally, I'd like to generate that part of my README from --list-all-steps.)

That aside, I find myself redefining a lot of commands to get rid of default flags I don't want, like --all-features on build commands, which could cause build errors if flags are meant to be mutually exclusive as in rust-cpython, and missing commands which one would think would be obvious to provide a coherent interface, such as a wrapper around cargo run so that makers or a shorter shell alias of it can become muscle memory and I can focus on the subcommands.

At present, since I can't find a way to omit the Makefile.toml in the README, my plan is to write a quick little script which takes an installed instance of cargo-make and my Makefile.toml and amends it with all the lines needed to manually cut out all the predefined bits I don't want.

(I already build this stuff from scratch. I'm only interested in cargo-make because it's starting to get more painful to hack around Just's myriad GNU Make-esque shortcomings than it would be to write said "explicitly disable all defaults" script.)

EDIT: I had an idiot moment. skip_core_tasks = true exists.

Describe the solution you'd like

Ideally, a Makefile.toml option which lets me whitelist and rename which tasks I receive from Makefile.toml (eg. I'm perfectly happy to let you maintain the kcov-related tasks), so Makefile.toml is less use defaults::* and more use defaults::coverage-kcov as kcov;

Failing that, I'll take a simple option in Makefile.toml which prevents the default Makefile.toml from being loaded so I can take full responsibility for writing and maintaining all of my tasks.

@sagiegurari
Copy link
Owner

are you looking for something like this?

https://github.com/sagiegurari/cargo-make#disabling-predefined-tasksflows

by the way, the reason i put all those predefined tasks internally is to enable the tool to be productive from the first minute and reduce the need to having projects copy pasting same tasks over and over.
you might disagree with how many of the tasks are built, but generally speaking I think its nice that I do not have to redefine tasks for every project and than maintain it and fix those in all projects if there is an issue or some new flag i want in and so on...

@sagiegurari
Copy link
Owner

can you explain more about this one?

Ideally, a Makefile.toml option which lets me whitelist and rename which tasks I receive from Makefile.toml (eg. I'm perfectly happy to let you maintain the kcov-related tasks), so Makefile.toml is less use defaults::* and more use defaults::coverage-kcov as kcov;

how you would like to see it in yoru toml file?

[config]
include_core_tasks = ["build*", "test*"]

something like that?

@ssokolow
Copy link
Author

are you looking for something like this?

*facepalm*

I looked right at that several times and, because I was to bed too late and awake too early today, every time I looked at that, I mistook it for the documentation on applying disabled = true to individual commands.

Sorry about that.

by the way, the reason i put all those predefined tasks internally is to enable the tool to be productive from the first minute and reduce the need to having projects copy pasting same tasks over and over.
you might disagree with how many of the tasks are built, but generally speaking I think its nice that I do not have to redefine tasks for every project and than maintain it and fix those in all projects if there is an issue or some new flag i want in and so on...

Yes, the problem is that, as a UI/UX guy, I have strong opinions about how an interface should work and, scrolling through Makefile.stable.toml, I find myself saying "I'll need to change that" or "I'll want to rename that command" or "I'll want to disable that so people don't think it's a supported action as implemented in the default" so often that it's easier to write a Makefile.toml from scratch and just use the default as a cookbook/reference for the proper way to accomplish certain tasks.

...similar to how I've decided I probably won't rebase my template on QuiCLI because it feels like, by the time I've wrestled it into behaving the way I want, the codebase will be 50% QuiCLI and then another 50% hacks to override its default behaviours.

(eg. Just StructOpt (not even QuiCLI's helpers for it) needs author="" and a \n on the beginning of the about string to match Linux platform conventions... sometimes with further consequences. For example, if you omit that \n, help2man will mistake the <name> <version> string for part of the about string.)

can you explain more about this one?

There are cases where I would like to use some of your default tasks without having all of them in --list-all-steps. However, the current names are inconsistent with my naming scheme and may collide with my own tasks in some cases.

For example:

  1. build should do something half-way between build and build-flow.
  2. I want to import your pre-* and post-* tasks so that, where appropriate, my tasks can share them with any predefined tasks I decide to use as-is.
  3. If I use a *-flow, I'd want it to have a name that better describes when I'd want to use it. For example, build-flow would probably be build-dist, because it's what I type when I want to make a build suitable for distribution without automatically publishing it.
[config]
include_core_tasks = ["build*", "test*"]

something like that?

It's a nice start, but I don't see how that syntax would allow me to rename included tasks so I could use the names you gave them for different tasks of my own.

@ssokolow ssokolow changed the title Support for completely disabling the default makefile Support for including only parts of the default makefile without risk of naming collisions Mar 12, 2019
@sagiegurari
Copy link
Owner

ok, let me think of something that is both simple to configure and meets the needs.
however, i believe that you might run into situation where you included task A which depends on B but B is not included.
in this case it will fail when running A unless you redefine B on your own.
does that make sense? otherwise i don't see how you can include only some tasks and expect them to work without their dependencies being included

@ssokolow
Copy link
Author

ssokolow commented Mar 12, 2019

I was thinking of it more like namespacing. That's why I likened it to use.

Under that interpretation, it'd be equivalent to:

  1. Include everything I don't ask for, but force private = true on it.
  2. If I define build without importing the default first, then the build that build-flow references would be internally known as something like default::build so that build-flow could still distinguish it from the build that I defined and call it instead. (This would be how I could both define my own build without causing random behavioural changes in the defaults and, at the same time, customize pre-* and post-* tasks.)

There's a reason I characterized the current state of things as use defaults::*; If I include a single item from some crate's prelude, it can still reference the things I didn't use. I just can't see them.

@sagiegurari
Copy link
Owner

if we 'namespace' the core tasks, i'm not sure renaming and excluding/including is needed.
What do you think about the following?

[config.modify_core_tasks]
private = true # if true, all core tasks are set to private (default false)
namespace = "default" # if set to some value, all core tasks are modified to: <namespace>::<name> for example default::build

This will give you MOST of what you are looking for except allowing you to export core tasks as is, in which case you will have to define your own tasks in your makefile and simply run the core namespaced task.
for example

[config.modify_core_tasks]
private = true
namespace = "default"

[tasks.build]
run_task = "default::build"

@ssokolow
Copy link
Author

That'd work nicely. :)

@sagiegurari
Copy link
Owner

@ssokolow I just pushed an initial version.
you can test it from the 0.17.0 development branch.

@sagiegurari sagiegurari added this to the 0.17.0 milestone Mar 16, 2019
@ssokolow
Copy link
Author

Glad to hear it. I'll try to fit testing it in today or tomorrow.

@ssokolow
Copy link
Author

I just tried using your suggested approach for mapping kcov to default::coverage-kcov. Here's what I've found so far:

  1. In hindsight, it's functioning as expected, but it's bothersome that I can't inherit things like description, category, and windows_alias when all I want to do is rename a command from the default set so it doesn't collide with something I defined.
  2. You'll want to clearly document the [tasks."default::pre-foo"] syntax needed to override namespaced tasks.
  3. Your coverage-kcov errors out when it encounters the copy of kcov 33 that I've been using with my homegrown kcov task because it blindly assumes the version number rather than calling --version to check.

(That last one is sufficient reason to copy-paste your coverage-kcov task and patch it, rather than relying on the default copy as I'd hoped I could do, because I've been putting a lot of effort into making this stuff Just Work™ lately. I'm actually getting close to finishing an automated test suite for the current justfile-based automation.)

@sagiegurari
Copy link
Owner

In hindsight, it's functioning as expected, but it's bothersome that I can't inherit things like description, category, and windows_alias when all I want to do is rename a command from the default set so it doesn't collide with something I defined.

actually you can. instead of doing

[tasks.mytask]
run_task = "default::core_task"

you can use aliases which copy everything with them but do not enable customizations.

[tasks.mytask]
alias = "default::core_task"

and you will have all fields the same as the original task.

You'll want to clearly document the [tasks."default::pre-foo"] syntax needed to override namespaced tasks.

Good idea

Your coverage-kcov errors out when it encounters the copy of kcov 33 that I've been using with my homegrown kcov task because it blindly assumes the version number rather than calling --version to check.

can you please share more details? do you mean that kcov is failing because of my usage of few args that are not supported in kcov 33?
if so... i can modify the installation script to ensure its at least 34 and if not run the full install script.

@ssokolow
Copy link
Author

you can use aliases which copy everything with them but do not enable customizations.

I'm not really a big fan of that idea, since I know I'll want to customize category on every task I expose to the user.

That said, it doesn't seem to be working anyway.

If I take a config which is successful with [tasks.kcov] containing run_task = "default::coverage-kcov" and change it to `alias = "default::coverage-kcov"`` without touching anything else, I get this error:

[cargo-make] INFO - Task: kcov
[cargo-make] INFO - Profile: development
[cargo-make] ERROR - Task not found: kcov
[cargo-make] WARN - Build Failed.

can you please share more details? do you mean that kcov is failing because of my usage of few args that are not supported in kcov 33?

That's correct.

Working Directory: /home/ssokolow/Documents/Critical/src/rust-cli-boilerplate
Running tests from directory: target/debug
Test binary filter regex: boilerplate-[a-z0-9]*$\|test_[a-z0-9]*-[a-z0-9]*$\|[a-z0-9]*_test-[a-z0-9]*$
Running coverage for file: target/debug/boilerplate-7be1e02d97c40c9f
kcov: unrecognized option '--exclude-line=kcov-ignore'
kcov: error: Unrecognized option: -

Usage: kcov [OPTIONS] out-dir in-file [args...]

Where [OPTIONS] are
 -h, --help              this text
 --version               print the version of kcov
 -p, --pid=PID           trace PID instead of executing in-file,
                         in-file is optional on Linux in this case
 -l, --limits=low,high   setup limits for low/high coverage (default 25,75)
 
 --collect-only          Only collect coverage data (don't produce HTML/
                         Cobertura output)
 --report-only           Produce output from stored databases, don't collect
 --merge                 Merge output from multiple source dirs
 
 --include-path=path     comma-separated paths to include in the coverage report
 --exclude-path=path     comma-separated paths to exclude from the coverage
                         report
 --include-pattern=pat   comma-separated path patterns to include in the
                         coverage report
 --exclude-pattern=pat   comma-separated path patterns to exclude from the
                         coverage report
 
 --coveralls-id=id       Travis CI job ID or secret repo_token for uploads to
                         Coveralls.io
 --strip-path=path       If not set, max common path will be stripped away.
 --uncommon-options      print uncommon options for --help

Examples:
  kcov /tmp/kcov ./frodo                        # Check coverage for ./frodo
  kcov --pid=1000 /tmp/kcov                     # Check coverage for PID 1000
  kcov --include-pattern=/src/frodo/ /tmp/kcov ./frodo # Only include files
                                                       # including /src/frodo
  kcov --collect-only /tmp/kcov ./frodo  # Collect coverage, don't report
  kcov --report-only /tmp/kcov ./frodo   # Report coverage collected above
  kcov --merge /tmp/out /tmp/dir1 /tmp/dir2    # Merge the dir1/dir2 reports

@sagiegurari
Copy link
Owner

I'm not really a big fan of that idea, since I know I'll want to customize category on every task I expose to the user.

if you are customizing the category, write a small description as well :)
I don't think thats a big issue.
i assumed you prefer to customize which is why i suggested the run_task

That said, it doesn't seem to be working anyway.

not fun. will check it and solve it as it should work and is part of this namespacing solution.

as for kcov, i opened a new issue on that #203

@ssokolow
Copy link
Author

ssokolow commented Mar 17, 2019

I don't mind customizing the description. My concern is other fields which may be relevant.

(eg. Suppose a default task has a configuration that flat-out hides it from the listing of available tasks on unsupported platforms. That'd be something that really should be automatically kept in sync with the implementation that such a limitation is based on.)

...or, to look at it from the other side, alias just seems so restrictive that I'd never use it for this kind of purpose in something I'd want to be seen as having authored.

@sagiegurari
Copy link
Owner

i could add 'extends' attribute for tasks (was thinking about it for some time, but never did it).
as part of another issue

@ssokolow
Copy link
Author

Assuming it works as I imagine, I could see that being useful for quite a bit more than just this case.

For example, defining a task with inherently complex behaviour like is present in coverage-kcov and then defining an extends task which extends it and modifies the behaviour by overriding an environment variable like CARGO_BUILD_TARGET or RUSTUP_TOOLCHAIN.

@sagiegurari
Copy link
Owner

small update regarding aliases. they do work just fine.
but if you set all core tasks to private and than do an alias, you can't trigger it from cli directly as it is basically private.

[config.modify_core_tasks]
private = true
namespace = "default"

# this task is actually now private so can't be invoked directly from cli
[tasks.empty2]
alias = "default::empty"

@sagiegurari
Copy link
Owner

extend now pushed to the 0.17.0 branch.
it doesn't yet implemented for list all steps command, but it does work good for actual cargo make task invocations in runtime.

@sagiegurari
Copy link
Owner

So far, all of the following were pushed to the 0.17.0 dev branch.
Which I believe sums up all your needs and a bit more.
Try it out and please tell me if you are missing anything.
All docs updated, which you can read in the 0.17.0 branch to understand how to work with all these.

@ssokolow
Copy link
Author

Will do. I'll try to get you a solid response by the end of tomorrow, my time.

@sagiegurari
Copy link
Owner

0.17.0 released with the changes. if you need more changes, please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants