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

.murdock: disable hash checks of kconfig/make #18423

Merged
merged 15 commits into from
Aug 17, 2022

Conversation

MrKevinWeiss
Copy link
Contributor

Much CI time is wasted as unrelated hash failure occur.
Finding out why is taking some time.
In order to stop killing dolphins we will disable only the hash checks.
There is a risk of introducing new issues with the kconfig/make
dependency resolution.
However, the package/module checks are still enforced which should catch
95% of the problems.
The nightlies will continue to check as well.

Contribution description

Testing procedure

Check the murdock output? It is pretty hard to test CI changes.

Issues/PRs references

A little less destructive than #18399

Much CI time is waisted as unrelated hash failure occur.
Finding out why is taking some time.
In order to stop killing dolphins we will disable only the hash checks.
There is a risk of introducing new issues with the kconfig/make
dependency resolution.
However, the package/module checks are still enforced which should catch
95% of the problems.
The nightlies will continue to check as well.
@MrKevinWeiss MrKevinWeiss requested a review from kaspar030 as a code owner August 9, 2022 08:44
@github-actions github-actions bot added the Area: CI Area: Continuous Integration of RIOT components label Aug 9, 2022
@MrKevinWeiss MrKevinWeiss requested a review from gschorcht August 9, 2022 08:44
@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 9, 2022
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Aug 9, 2022
@leandrolanzieri
Copy link
Contributor

Looks like you caught a difference in the module list that was not impacting the binary hash

@MrKevinWeiss
Copy link
Contributor Author

I imagine there are a few of these.

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Aug 11, 2022
@MrKevinWeiss
Copy link
Contributor Author

It seems the failures are caused by periph_gpio_ll_irq_unmask included in every make build. The seems a bit wrong so I adapted the make to only bring it in when the periph_gpio_ll. I would expect that kconfig will still have some issues with some apps but I guess we will see.

Introduced in #16787, ping @maribu if you want to confirm my off-the-cuff logic.

@leandrolanzieri
Copy link
Contributor

It seems the failures are caused by periph_gpio_ll_irq_unmask included in every make build. The seems a bit wrong so I adapted the make to only bring it in when the periph_gpio_ll. I would expect that kconfig will still have some issues with some apps but I guess we will see.

Introduced in #16787, ping @maribu if you want to confirm my off-the-cuff logic.

I think this may be more complicated, as the periph_gpio_ll and related modules were not modelled in Kconfig when introduced in #16787

@github-actions github-actions bot added the Area: build system Area: Build system label Aug 12, 2022
@MrKevinWeiss MrKevinWeiss requested a review from jia200x as a code owner August 12, 2022 12:08
@github-actions github-actions bot added Area: Kconfig Area: Kconfig integration Area: sys Area: System labels Aug 12, 2022
This creates diff failures when calling info-modules and info-packages.
Ideally info-modules and info-packages have clean output for the CI build
@leandrolanzieri leandrolanzieri added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Aug 17, 2022
@MrKevinWeiss MrKevinWeiss force-pushed the pr/disable/hashchecks branch from ea9a356 to a00060f Compare August 17, 2022 12:37
@MrKevinWeiss
Copy link
Contributor Author

As this prevents the hash failures we should not skip this and rather enable automerge so we will have it in some time tonight.

@leandrolanzieri leandrolanzieri merged commit 2dd5923 into RIOT-OS:master Aug 17, 2022
@MrKevinWeiss MrKevinWeiss deleted the pr/disable/hashchecks branch August 17, 2022 21:36
@MrKevinWeiss
Copy link
Contributor Author

Thank you for the review

MrKevinWeiss added a commit to MrKevinWeiss/RIOT that referenced this pull request Aug 18, 2022
After introducing RIOT-OS#18423 there are occasional messages that still happen.
These messages cause a diff output when testing with TEST_KCONFIG=1.
This then causes a failure when comparing make/kconfig modules and packages.
MrKevinWeiss added a commit to MrKevinWeiss/RIOT that referenced this pull request Aug 18, 2022
After introducing RIOT-OS#18423 there are occasional messages that still happen.
These messages cause a diff output when testing with TEST_KCONFIG=1.
This then causes a failure when comparing make/kconfig modules and packages.
MrKevinWeiss added a commit to MrKevinWeiss/RIOT that referenced this pull request Aug 18, 2022
After introducing RIOT-OS#18423 there are occasional messages that still happen.
These messages cause a diff output when testing with TEST_KCONFIG=1.
This then causes a failure when comparing make/kconfig modules and packages.
MrKevinWeiss added a commit to MrKevinWeiss/RIOT that referenced this pull request Aug 22, 2022
After introducing RIOT-OS#18423 there are occasional messages that still happen.
These messages cause a diff output when testing with TEST_KCONFIG=1.
This then causes a failure when comparing make/kconfig modules and packages.
MrKevinWeiss added a commit to MrKevinWeiss/RIOT that referenced this pull request Aug 22, 2022
After introducing RIOT-OS#18423 there are occasional messages that still happen.
These messages cause a diff output when testing with TEST_KCONFIG=1.
This then causes a failure when comparing make/kconfig modules and packages.
@maribu maribu added this to the Release 2022.10 milestone Oct 14, 2022
dylad added a commit to dylad/RIOT that referenced this pull request Dec 1, 2022
This was partially cleans up in RIOT-OS#18423 but it looks like this one was missed.

Signed-off-by: Dylan Laduranty <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: CI Area: Continuous Integration of RIOT components Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration Area: sys Area: System Area: tests Area: tests and testing framework CI: full build disable CI build filter CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants