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

Feature/misc meson improvements #1992

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

perigoso
Copy link
Contributor

@perigoso perigoso commented Nov 4, 2024

Detailed description

This is a minor cleanup/simplification of the meson configuration around the target enabling

This should result in no behavior changes, except for a small, but very intentional one.

Right now when manually configuring the build, i.e. not using the pre-made cross-files for the probes, the default behaviour was to enable all targets by default, resulting in the need to safe-guard against special cases for certain probes to reduce user confusion.

This changes things such that by default when manually configuring the build no targets are enabled by default, a helpful error message should be displayed about it (improvement over the previous compilation error).

I think this makes sense because this is to me an "advanced user" usage and reduces the maintenance burden on the build system.

Your checklist for this pull request

Closing issues

@perigoso
Copy link
Contributor Author

perigoso commented Nov 5, 2024

Whoops, i missed that this needed rebasing

@perigoso perigoso force-pushed the feature/misc-meson-improvements branch 2 times, most recently from fcd82a1 to 8aedc0d Compare November 5, 2024 15:52
Previously when configuring the build for a probe manually, the default would be to enable all debug targets
This could lead to user confusion as the build would fail due to lack of space for some probes, for this reason an edge case condition was added to override the defaults, the method employed does have a problems, it prevents enabling all targets explicitly for the affected probems, even if the user knows what they are doing, and increases the maintenance burden as a list of enabled targets needs to be maintained in multiple places

This makes it so the default for manually configured builds (i.e. not using the pre-made cross-files) is to not enable *any* debug targets, they must be configured manually, this gives a clear warning if one would forget to do this, and removes the maintenance burden with duplicate lists
This automatically adds all targets to the libbmd dependencies removing the need to manually update this list when new targets are added
although it is tied to the debug target configuration being dependent on riscv_jtag_dtm, this option just sets a define and is better grouped with other similar configuration options
store the dependency objects themselves instead of strings
massage the target declarations to remove conditional targets based on which targets are enabled
after a small rewrite the name of the target options changed slightly, meaning more targets were enabled by default than before
this commit adjusts this to retain the previous behavior
@perigoso perigoso force-pushed the feature/misc-meson-improvements branch from 8aedc0d to 73a4cb1 Compare November 7, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant