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

pkg/tensorflow-lite: blacklist esp32 architecture [backport 2020.01] #13150

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jan 17, 2020

Backport of #13140

Contribution description

As proposed in #13133 (comment), this PR blacklists the ESP32 architecture in the tensorflow-lite package.

Testing procedure

  • Check that pkg/tensorflow-lite build is forbidden by the build system on esp32 architecture:
BUILD_IN_DOCKER=1 make BOARD=esp32-wroom-32 -C tests/pkg_tensorflow-lite
make: Entering directory '/work/riot/RIOT/tests/pkg_tensorflow-lite'
ESP32_SDK_DIR should be defined as /path/to/esp-idf directory
ESP32_SDK_DIR is set by default to /opt/esp/esp-idf
Some feature requirements are blacklisted: arch_esp32
/work/riot/RIOT/tests/pkg_tensorflow-lite/../../Makefile.include:843: *** You can let the build continue on expected errors by setting CONTINUE_ON_EXPECTED_ERRORS=1 to the command line.  Stop.

Issues/PRs references

This is a workaround for the problem reported in #13133 on ESP32 (details in #13133 (comment))

@aabadie aabadie added Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 17, 2020
Copy link
Contributor

@gschorcht gschorcht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A straightforward solution as tested in the master.

fjmolinas
fjmolinas previously approved these changes Jan 17, 2020
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

@aabadie aabadie force-pushed the backport/2020.01/pr/pkg/tensorflow-lite-blacklist-esp32 branch from 9b65a64 to bdfb39f Compare January 17, 2020 11:40
@aabadie
Copy link
Contributor Author

aabadie commented Jan 17, 2020

@fjmolinas, it seems that your ACK was dismissed when I force pushed the updated branch (the branch must be up to date with the release branch..., btw why this? )

@fjmolinas
Copy link
Contributor

(the branch must be up to date with the release branch..., btw why this? )

Rebased on the release branch? No idea.

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK

@aabadie
Copy link
Contributor Author

aabadie commented Jan 17, 2020

Rebased on the release branch? No idea.

In fact, each backport PR in the queue must be rebased when another one is merged in the meantime. This is quite annoying.

@kaspar030
Copy link
Contributor

In fact, each backport PR in the queue must be rebased when another one is merged in the meantime. This is quite annoying.

It is, but it ensures that CI tested the actual result of a merge to the current release branch HEAD. Without this, there are merge interaction race conditions.

@aabadie
Copy link
Contributor Author

aabadie commented Jan 17, 2020

It is, but it ensures that CI tested the actual result of a merge to the current release branch HEAD

Agreed, and I'm not against this. It is annoying because there are 3 backport PRs in the queue and Murdock takes 30m to build each PRs including random PRs that target master.

Would it be possible to reorder the priority of backport PRs in the Murdock queue ?

@kaspar030
Copy link
Contributor

Would it be possible to reorder the priority of backport PRs in the Murdock queue ?

Not at the moment.

@aabadie
Copy link
Contributor Author

aabadie commented Jan 17, 2020

Not at the moment.

No problem, but would be a great feature to add to Murdock ;)

@aabadie aabadie merged commit ca62281 into RIOT-OS:2020.01-branch Jan 17, 2020
@aabadie aabadie deleted the backport/2020.01/pr/pkg/tensorflow-lite-blacklist-esp32 branch January 17, 2020 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants