[RFC] build profile proposal

Fabien-Chouteau commented 3 years ago

The goal here is to provide a simpler default solution for handling compilation flags in Alire crates. And offer a more accessible understanding of compilation options for the vast majority of users.

This feature is intended to be recommended, to encourage a homogeneous development experience, but not mandatory. Developers can still maintain their own set of switches in custom GPR files. They can also control run-time checks in the source code with the pragmas Suppress, Unsuppress, as well as style checks with the pragma Style_Checks.

[edit: s/standard/default] [edit: Add debug info category] [edit: take switches from external ADAFLAGS] [edit: split build_profiles and build_switches]

Switches categories:

The switches categories define what are the different aspect of compilation that can be controlled. They are enumeration types.

For instance we can define the following categories and their values:

Optimization: performance, size, debug
Debug info: none, all
Run-time checks: none, default, overflow, all
Run-time contracts: none, all
Compile checks: none, warnings, errors
Style checks: none, default

(See below for the corresponding GNAT switches)

Profiles:

Profiles are sets of values for the categories. They define the full scope of compilation switches for a given situation.

For instance we can define the following profiles:

Release
- Optimization => performance
- Debug info => none
- Run-time checks => default
- Run-time contracts => none
- Compile checks => none
- Style checks => none
Validation
- Optimization => performance
- Debug info => all
- Run-time checks => all
- Run-time contracts => all
- Compile checks => errors
- Style checks => default
Development
- Optimization => debug
- Debug info => all
- Run-time checks => all
- Run-time contracts => all
- Compile checks => warnings
- Style checks => default

Default Profile

When working on a crate, the dependency crates are compiled with the Release profile, and the root crate is compiled with Development profile. It is possible to override the profile of the root crate with an alr switch (to be define):

alr build --release
alr build --validation

Customize Profiles

Crates can modify the default switches of profiles. For instance a crate that is written and proved with SPARK can safely disable all run-time checks:

[build_switches]
release.runtime_checks = "none"
release.contracts = "none"

It is also possible to change actual switches:

[build_switches]
validation.style = ["-gnatyg", "-gnatM120"]

Or with a wildcard, change the switches for all profiles:

[build_switches]
"*".style = ["-gnatyg", "-gnatM120"]

Setting build profiles and switches of dependencies

If one wants to investigate an issue with a dependencies, for instance the clic crate, it is possible to override the default profile:

[build_profile]
clic = "developement"

If one wants maximum safety with all run-time checks and contracts for the dependencies, the wildcard "*" can be used to change the switches of all dependencies:

[build_switches]
crate."*".release = { "runtime_checks" = "all", "contracts" = "all"}

For an embedded project, one might want to optimize the code size:

[build_switches]
crate."*".release = { "optimization" = "size"}

In practice

Using a similar approach as the crate configuration system. The set of compiler switches for each crate will be written in the config\<CRATE>_config.gpr project file:

   Ada_Compiler_Switches := External_As_List ("ADAFLAGS", " ") & ("-O3", "-gnatn", "-gnatp", "-gnato");

The main project file can then use this value:

with "config/clic_config.gpr";

project CLIC is

   package Compiler is
      for Switches ("Ada") use CLIC_Config.Compiler_Switches;
  end Compiler;
end CLIC;

Switches for GNAT

- Optimization:
  - Performance: [-O3,    -- Optim level 3
                  -gnatn, -- Enable inlining
                 ]
  - Size: [-Os,                 -- Optim size
           -fdata-sections,     -- Place each data into its own section
           -ffunction-sections, -- Place each function into its own section 
          ]
  - Debug: [-Og,    -- Optim for debug
           ]
- Debug info:
  - None: []
  - All: [-g]
- Run-time checks:
  - None: [-gnatp] -- Suppress all checks
  - default: []
  - Overflow: [-gnatp, -- Suppress all checks
               -gnato] -- Enable overflow checks
  - All: [-gnato]  -- Enable overflow checks
- Run-time contracts:
  - None: []
  - All: [-gnata]
- Compile checks:
  - None: []
  - Warnings: [-gnatwa, -- All warnings
               -gnatVa, -- All validity checks
              ]
  - Errors: [-gnatwa, -- All warnings
             -gnatVa, -- All validity checks
             -gnatwe, -- Warnings as errors
            ]
- Style check:
  - None: []
  - Standard: [-gnaty3,  -- Specify indentation level
               -gnatya,  -- Check attribute casing
               -gnatyA,  -- Use of array index numbers in array attributes
               -gnatB,   -- Blanks not allowed at statement end
               -gnatyb,  -- Check Boolean operators
               -gnatyc,  -- Check comments, double space
               -gnatyD,  -- Check declared identifiers in mixed case
               -gnaty-d, -- Allow DOS line terminators
               -gnatye,  -- Check end/exit labels
               -gnatyf,  -- No form feeds or vertical tabs
               -gnatyh,  -- No horizontal tabs
               -gnatyi,  -- Check if-then layout
               -gnatyI,  -- check mode IN keywords
               -gnatyk,  -- Check keyword casing
               -gnatyl,  -- Check layout
               -gnatym,  -- Check maximum line length
               -gnatyn,  -- Check casing of entities in Standard
               -gnatyO,  -- Overriding subprograms explicitly marked as such
               -gnatyp,  -- Check pragma casing
               -gnatyr,  -- Check references
               -gnatyS,  -- Check no statements after then/else
               -gnatyt,  -- Check token spacing
               -gnatyu,  -- Check unnecessary blank lines
               -gnatyx,  -- Check extra parentheses
              ]

jklmnn commented 3 years ago

I would add -d to the default Debug profile in Optimization.

Fabien-Chouteau commented 3 years ago

I would add -d to the default Debug profile in Optimization.

gprbuild's -d Display compilation progress? If yes, then it's not a compiler option, so outside the scope of this RFC.

Note that with the next release of Alire you will be able to pass switches to gprbuild like so: alr build -- -d

JeremyGrosser commented 3 years ago

I think the word standard should be changed to default or normal, as those style checks are not a part of any Ada standard document.

Are the -gnat* flags really preferred over a .adc file with a bunch of more readable pragmas?

Glacia commented 3 years ago

Imo, debug information should be it's own category, you should be able to make a build with optimizations and debug info on.

Fabien-Chouteau commented 3 years ago

I think the word standard should be changed to default or normal, as those style checks are not a part of any Ada standard document.

Indeed.

Are the -gnat* flags really preferred over a .adc file with a bunch of more readable pragmas?

.adc covers some of the switches but not all, and I don't want to to split things and implement two systems. People can still use a .adc files if they want to.

jklmnn commented 3 years ago

I would add -d to the default Debug profile in Optimization.

gprbuild's -d Display compilation progress? If yes, then it's not a compiler option, so outside the scope of this RFC.

Note that with the next release of Alire you will be able to pass switches to gprbuild like so: alr build -- -d

I meant -g for debug symbols in gcc, sorry for the confusion.

Fabien-Chouteau commented 3 years ago

I meant -g for debug symbols in gcc, sorry for the confusion.

As @Glacia suggested, we should probably have a category for debug info. Editing now.

mosteo commented 3 years ago

Super important addition. Great summary, Fabien.

Should we have some mechanism in the global config for users to tweak their defaults? Maybe a [build_profile] that is copied by default during alr init if existing?

Fabien-Chouteau commented 3 years ago

Should we have some mechanism in the global config for users to tweak their defaults? Maybe a [build_profile] that is copied by default during alr init if existing?

Configuration could be used, but I am afraid that it will be too easy to make mistakes like having a crate that doesn't compile if the user do not put the right stuff in their config.

Fabien-Chouteau commented 2 years ago

Quick update:

I made a first prototype in this branch that supports:

[[depends-on]]
lib_b = "~0.0.0"

[build_profile]
lib_b = "development"
"*"   = "validation"

[build_switches]
release.runtime_checks = "yes"
release.contracts = "yes"
release.optimization = "size"
"*".style_checks = ["-gnatyg", "-gnatM120"]
validation.optimization = "Debug"
development.optimization = "Size"

etc.

So a crate can modify its own switches configuration from the default. And a root crate can change the "build profile" of its dependencies (forbidden in release manifests).

What is missing from the original proposal is the ability for a crate to change the switches configuration of its dependencies (not only the profile but the switches themselves). Right now I am actually not sure if it is a good idea to let crates do that...

mosteo commented 2 years ago

I've been playing a bit with the latest branch. To be frank, I don't have much feedback at this time, besides that it works as advertised :-)

Right now I am actually not sure if it is a good idea to let crates [change dependency switches]

I can envision a situation in which a dependency has a mistake and has to be overridden e.g. for a less used profile. So you have to resort to use another profile or manually edit its project files. At that point it could be convenient to be able to directly override from the root manifest. OTOH, I guess for dependencies not following profiles you're on your own anyway

In short, I'm not sure. I guess this is a feature where practical use will reveal the necessary tweaks.

One question: as the profile is selected in the config file, I assumed that disabling configuration will disable updates to profiles, and my tests confirm this. Do we want to tie both features (config and profiles)? I guess so, and this is a deliberate design decision.

I still have to play with overrides for parts of profiles, I've only tested per-crate profiles.

Fabien-Chouteau commented 2 years ago

One question: as the profile is selected in the config file, I assumed that disabling configuration will disable updates to profiles, and my tests confirm this. Do we want to tie both features (config and profiles)? I guess so, and this is a deliberate design decision.

I don't know how to implement the profiles other than by adding switches in the GPR generated for crate config. I added a way to disable crate config in case it would override existing an config dir or its content for existing project, but I really hope that people will always use this feature for their crates.

mosteo commented 2 years ago

I really cannot think of a simple way of untying both besides going for separate project files, and I concur that isn't worth the trouble at least unless clear demand arises

Fabien-Chouteau commented 2 years ago

So we now have the basic build profile and switch support in the dev branch.

There is one thing I would like to tackle but I am not sure how handle.

In the current implementation, the default profile for the root crate is always development, this can be changed by changing the root manifest [1].

I wanted to have a way to also change this from the command line (similar to Cargo's cargo build --release). The issue for us is that the profile is actually set in the generate gpr file for the crate configuration, and so far, this file is only generated (or re-generate) when the manifest changes.

I started a implementation in the alr build command that would accept a --release switch and regenerate the crate configuration files when it is used. This works ok, but following run of alr build will keep this mode even when the switch is not used, or even worst will have the build mode change depending on commands used in between:

$ alr build --release # build in release mode
$ ald build # build in release mode
$ alr with aaa
$ alr build # build in development mode...

One solution would be always generate the crate configuration files when building, whether or not a build mode switch is used. I am not sure of the performance penalty of re-generating the files every time...

Could the lockfile be used for this?

Thoughts?

[1]

[build-profile]
my_crate = "release"

(CC @mosteo)

mosteo commented 2 years ago

I understand the problem...

Could the lockfile be used for this?

I think so, as the worst that can happen is that if the lockfile is deleted the build mode would revert to the default. I think this could be a good solution indeed to avoid always regenerating, which can have other unintended consequences because of the file always changing timestamp.

This will entail a few tweaks as currently we are caching the solution in the root to avoid costly lockfile reloads. Instead, the whole contents of the lockfile would have to be cached instead, which would be the better thing to do, precisely because we may want to store other things there. If you don't want to deal with this unrelated code smell let me know and I could fix it before you put the build mode into the lockfile, and it will make things cleaner for you.

Fabien-Chouteau commented 2 years ago

Or maybe the local config file could be used here to log the latest build profile...

mosteo commented 2 years ago

Or maybe the local config file could be used here to log the latest build profile...

Indeed, it seems even a better option.

mosteo commented 4 months ago

Available since 1.2

alire-project / alire