New switches for alr build to apply profile to all dependencies

[mosteo]

Two new switches that allow to apply a profile to the complete solution without having to edit the manifest:

• alr build --validation --recurse-all # Overrides all crates
• alr build --validation --recurse-unset # Overrides only crates without explicit settings in manifests

These require an explicit profile to apply.

The use case is when you want to e.g. make a one-shot full debug/validation build.

[Fabien-Chouteau]

I am ok on the feature in principle, I am not sure about switches.

Why not use existing switches with an extra suffix:

• --validation-all
• --development-all
• --release-all (This one is not so useful as it would be the same as --release)

[mosteo]

I prefer this way because it seems more compact/maintainable than having three varieties per profile (when counting the -unset variation), it's 5 vs 9 switches. Also if in the future we decided some new profile makes sense (profiling?). That said, I'm not totally happy with the recurse part of the name, but I'm out of ideas.

(Minor aside: --release-all would still be useful, as that one would override a dependency profile that is explicitly set in some manifest. With these new switches, the priorities go: default -> manifest -> command-line.)

[Fabien-Chouteau]

--recurse-all could be understood as "build all dependencies recursively"?

[mosteo]

Right. And recurse-profile-all could be interpreted as something about profiling. I think we need a creative departure here.

Also I just saw in the help for alire build that it talks about build modes and not profiles.

--apply-mode [all|unset]

--apply-build-mode

--propagate

--propagate-mode

--propagate-profile

I checked cargo build and they have cargo build --release and cargo build --profile <name>. It seems to apply to everything that's built (although the documentation is not very explicit). They default to dev build for everything in cargo build and release build in cargo install.

Perhaps we could reconsider our alr build defaults. For me it's counterintuitive that by default dependencies are built in release mode, as you lose many of the expected Ada features (range checks, numeric checks, etc...) But that would be for another PR.

[Fabien-Chouteau]

Perhaps we could reconsider our alr build defaults. For me it's counterintuitive that by default dependencies are built in release mode, as you lose many of the expected Ada features (range checks, numeric checks, etc...) But that would be for another PR.

You can also loose a lot in terms of performances and code size. I think release mode is a safest default for the future of the project as the number of dependencies grows.

@onox you are working on a set of crates all at once, and I guess you want them all in debug mode. So a different default would be better for your use case, but I am not convinced that it would be good for everyone.

How about a --build-profiles switch with an argument that is a comma separated list of CRATE_NAME=BUILD_PROFILE. Like in the manifest. We can even introduce globing for the crate names: --build-profiles="something_*=Release"

[onox]

You can also loose a lot in terms of performances and code size. I think release mode is a safest default for the future of the project as the number of dependencies grows.

But if a user chooses --validation or --development, hasn't the user already accepted that performance isn't going to be great?

As @mosteo said, it's counterintuitive that the dependencies are always built in release mode, and I think it violates the law of least astonishment. From the output of alr help build it isn't clear that only the crate of the user is built with the given mode. The output does say "root crate", but how many users will realize that excludes the dependencies?

How about a --build-profiles switch with an argument that is a comma separated list of CRATE_NAME=BUILD_PROFILE. Like in the manifest. We can even introduce globing for the crate names: --build-profiles="something_*=Release"

This would be cumbersome to a user if the crate has many dependencies that cannot be catched by a glob pattern. It would also create too much flexibility for the problem of whether the build mode applies only to the crate itself or also to its dependencies.
A single switch to toggle between these two would be simpler and easier to understand for users. (Although cargo's behavior makes more sense to me)

Perhaps we should seek the feedback of a number of users? Here, on the Discussions page, or on gitter.

[mosteo]

First of all, I was wrong in assuming that the current release mode suppress any checks as far as I can see. It's pretty much only -O3 and -gnatn. I had assumed -gnato0 and -gnatp to be there. So what I was going to send doesn't really apply:

This issue reminds me of the -gnato affair. It was omitted in the default compiler config because of performance and so by default GNAT was not Ada conformant, which puzzled people regularly.

(Related: cargo has a benchmark profile. Perhaps that's the one where such omissions should be introduced.)

That said, I'd like simply to have an easy way of having a full build with the same profile everywhere without messing with manifests. I'm okay with keeping the current default.

I'm currently leaning to the --propagate-profile-{all|unset} nomenclature.

I've created this poll: #1140
I will mention it on gitter and telegram

[mosteo]

Okay, so after playing a bit more with the switches I wanted, and some internal changes now needed to ensure consistency of generated files between builds, and remembering that we have the aliases feature, I found the simplest and less confusing change is along the lines of @Fabien-Chouteau suggestion. So after my last push there's a single extra switch --profiles that allows both wildcard and named overriding:

`alr build --profiles '*:development,some_crate:validation'

Also I have added two aliases, alr build-dev and alr build-validation, that set all profiles. I omitted alr build-release as that's equivalent to alr build --release.

There's a second wildcard, '%', that sets only the crates without an explicit setting in a manifest (the unset variety of former switches).

The poll as I write gives a 30/70 split in expectations against the current behavior. If we were to change it, then the current behavior could be obtained with alr build --development --profiles '%:release', which could be given an alias instead. But changing established behaviors is always risky, and there can't be confusion now after looking at alr help build, so I've kept the current situation.

[Fabien-Chouteau]

`alr build --profiles '*:development,some_crate:validation'

That's the best option in my opinion. We can also accept shortcut names like dev = development to make things easier.

So after my last push there's a single extra switch --profiles

I think we should keep at least --validation and --release as these are very common options to use in CI scripts.
And you definitely don't want --validation to be applied to all your dependencies.

Also I have added two aliases, alr build-dev and alr build-validation, that set all profiles. I omitted alr build-release as that's equivalent to alr build --release.

I don't think that aliases are the way to go here, the switches are short enough.

There's a second wildcard, '%', that sets only the crates without an explicit setting in a manifest (the unset variety of former switches).

I am not sure this is worth the extra complexity. In my opinion people will either set profile in the manifest or on the command line, using both seems like a corner case.

The poll as I write gives a 30/70 split in expectations against the current behavior. If we were to change it, then the current behavior could be obtained with alr build --development --profiles '%:release', which could be given an alias instead. But changing established behaviors is always risky, and there can't be confusion now after looking at alr help build, so I've kept the current situation.

Changing the default behavior is a no-go for me. Building everything in dev mode might be ok in some situations but it won't scale up, we have to be future proof.

[Fabien-Chouteau]

it's counterintuitive that the dependencies are always built in release mode, and I think it violates the law of least astonishment

We are not equal in the astonishment department :)

I would be astonished to see warnings and style checks for dependencies I don't care about. Or not being able to have my embedded code fit in flash memory because dependencies of dependencies of my dependencies are compiled in debug mode.

This would be cumbersome to a user if the crate has many dependencies that cannot be catched by a glob pattern.

So what about the latest proposal?
I don't see alr build --profile '*:dev' being more cumbersome than alr build --development.

[mosteo]

I think we should keep at least --validation and --release as these are very common options to use in CI scripts.
And you definitely don't want --validation to be applied to all your dependencies.

These switches remain; only --profiles has been added.

I don't think that aliases are the way to go here, the switches are short enough.

Not a sticking point. One can define their own aliases if it proves too tiresome anyway.

I am not sure this is worth the extra complexity. In my opinion people will either set profile in the manifest or on the command line, using both seems like a corner case.

This is eyeing cases in which crates themselves known that some profiles won't work as-is and define their own override in the manifest. If they're deep in your dependencies it can be helpful if you want a "mostly"-homogeneous build. And, for example, our current default behavior couldn't be replicated otherwise in other combos (e.g. root in validation mode plus dependencies in dev mode, but respecting manifests).

I'd say it's worthy and not that complex internally, just an extra map.

Changing the default behavior is a no-go for me. Building everything in dev mode might be ok in some situations but it won't scale up, we have to be future proof.

I'd say the current status gives us all what we want in a clear way, and behavior when not using --profiles is still the same.

[Fabien-Chouteau]

Why not = for assign?

[mosteo]

I was in a mindset of mappings, JSON-style, and it seemed a bit less ugly than --profiles=crate=profile. I'm trying to remember if we have something similar in another place to follow convention. It's true that TOML uses = for maps...

[Fabien-Chouteau]

Duplicates of constants in Alire.Crate_Configuration.

[mosteo]

Thanks @Fabien-Chouteau and everyone for the involvement with this feature.