Skip to content

Fix workflows for new mbedtls #635

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

peterharperuk
Copy link
Contributor

Anything that builds picotool now has to pass "recursive" flag to the "git submodule init" command

Anything that builds picotool now has to pass "recursive" flag to the
"git submodule init" command
@will-v-pi
Copy link
Contributor

This shouldn't be needed once raspberrypi/picotool#226 is merged

@peterharperuk
Copy link
Contributor Author

I was wondering if we should go the other way and make more use of the mbedtls cmake files, although I haven't investigated if this is possible.

@will-v-pi
Copy link
Contributor

One benefit of moving to the SDK style is it reduces the number of files picotool compiles, as it doesn't need the x509 or TLS files so can skip those lists, whereas I'm not sure if there's a way to do that when using the mbedtls cmake files?

But the disadvantage is obviously maintenance, as we need to update the source lists if the sources change

@peterharperuk
Copy link
Contributor Author

reduces the number of files picotool compiles

Yes - that's a good reason!

Copy link
Contributor

@will-v-pi will-v-pi left a comment

Choose a reason for hiding this comment

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

Probably good to merge this anyway

@lurch
Copy link
Contributor

lurch commented Apr 15, 2025

pass "recursive" flag to the "git submodule init" command

I'm slightly wary of doing that (and yes I'm aware that you've already done that in some of the recent pico-sdk PRs, e.g. raspberrypi/pico-sdk#2416 ) because I'm worried about a situation like we previously had with tinyUSB cropping up again (where a 3rd-party repo had a lot of unnecessary submodules that we didn't need, which made the recursive git clone really slow). It's obviously not directly relevant to pico-sdk / pico-examples, but for an example of a repo with a LOT of submodules, see e.g. https://github.com/micropython/micropython/tree/master/lib

IMHO it's better to explicitly only init the submodules that we actually need, rather than recursively init-ing ALL submodules. However I'm happy to adopt the "let's leave it for now, and cross that bridge if we get to it" approach 🙂 🤞

@kilograham
Copy link
Contributor

pass "recursive" flag to the "git submodule init" command

I'm slightly wary of doing that (and yes I'm aware that you've already done that in some of the recent pico-sdk PRs, e.g. raspberrypi/pico-sdk#2416 ) because I'm worried about a situation like we previously had with tinyUSB cropping up again (where a 3rd-party repo had a lot of unnecessary submodules that we didn't need, which made the recursive git clone really slow). It's obviously not directly relevant to pico-sdk / pico-examples, but for an example of a repo with a LOT of submodules, see e.g. https://github.com/micropython/micropython/tree/master/lib

IMHO it's better to explicitly only init the submodules that we actually need, rather than recursively init-ing ALL submodules. However I'm happy to adopt the "let's leave it for now, and cross that bridge if we get to it" approach 🙂 🤞

yes, recursive init is bad anywhere users might copy, which workflows aren't, but there they can be bad because it takes extra time - better to explicitly checkout

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.

4 participants