Skip to content

Conversation

@utzig
Copy link
Member

@utzig utzig commented Feb 14, 2020

This adds support for repos to inform which submodules are required for a proper build on Mynewt. The main driver for this feature is that the current ext/mbedtls on mcuboot is a submodule that is not required by Mynewt builds, and the amount of submodules is going to grow in the future, eg, mcu-tools/mcuboot#653.

The way this works is newt is going to look for a "repo.submodules" key inside the "repository.yml" file. This is a list of submodules to be cloned. One of two things could happen:

  1. If the key is no present newt is going to clone all submodules (to preserve compatibility).
  2. If the key is found, each of the submodules declared will be cloned one by one.

For example, if "repository.yml" has the following contents:

repo.submodules: ""

In this case submodule cloning is skipped (with a verbose message stating it).

repo.submodules:
    - "ext/mbetls"
    - "boot/cypress/somemodule"

This will clone the two modules declared but not other submodules that might eventually exist.

This PR also makes cloning submodules recursive, so submodules inside submodules are also cloned.

@utzig utzig requested a review from ccollins476ad February 14, 2020 16:37
@utzig utzig force-pushed the repo-submodule-config branch from e94a29f to 14a8c71 Compare February 15, 2020 13:52
Copy link
Contributor

@ccollins476ad ccollins476ad left a comment

Choose a reason for hiding this comment

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

Looks good to me. I just had one minor comment that you can accept or ignore at your discretion.

// Check that there is a list of submodules to clone; if the key is
// not present, assume that all submodules should be cloned, otherwise
// only those configured will be cloned (or none if "")
func (r *Repo) loadSubmoduleConfig(yc *ycfg.YCfg) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Other functions that take a ycfg.YCfg object take it by value, not reference. It might make sense to change this function to match the others.

After programming in Go for several years, I still have no idea when to use pointers and when not to, so I am not saying pass-by-value is "better" here, just more consistent :).

Copy link
Member Author

Choose a reason for hiding this comment

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

Other functions that take a ycfg.YCfg object take it by value, not reference. It might make sense to change this function to match the others.

Done, thanks!

@utzig utzig force-pushed the repo-submodule-config branch from 14a8c71 to 0b9306e Compare February 19, 2020 20:09
This adds support for repos to inform which submodules are required for
a proper build on Mynewt. The main driver for this feature is that the
current `ext/mbedtls` on mcuboot is a submodule that is not required by
Mynewt builds, and the amount of submodules is going to grow in the
future, eg, mcu-tools/mcuboot#653.

The way this works is `newt` is going to look for a "repo.submodules"
key inside the "repository.yml" file. This is a list of submodules to be
cloned. One of two things could happen:

1) If the key is no present `newt` is going to clone all submodules (to
   preserve compatibility).
2) If the key is found, each of the submodules declared will be cloned
   one by one.

For example, if "repository.yml" has the following contents:

```
repo.submodules: ""
```

In this case submodule cloning is skipped (with a verbose message
stating it).

```
repo.submodules:
    - "ext/mbetls"
    - "boot/cypress/somemodule"
```

This will clone the two modules declared but not other submodules that
might eventually exist.

This PR also makes cloning submodules recursive, so submodules inside
submodules are also cloned.

Signed-off-by: Fabio Utzig <utzig@apache.org>
@utzig utzig merged commit 905c71c into apache:master Feb 20, 2020
@utzig utzig deleted the repo-submodule-config branch February 20, 2020 09:33
utzig added a commit to utzig/mcuboot that referenced this pull request Feb 25, 2020
A recently added `newt` feature allows it to only clone selected git
submodules: apache/mynewt-newt#377. This changes
the MCUBoot repository to remove submodules from the cloning process,
because they are not used by Mynewt.

Signed-off-by: Fabio Utzig <utzig@apache.org>
utzig added a commit to mcu-tools/mcuboot that referenced this pull request Feb 25, 2020
A recently added `newt` feature allows it to only clone selected git
submodules: apache/mynewt-newt#377. This changes
the MCUBoot repository to remove submodules from the cloning process,
because they are not used by Mynewt.

Signed-off-by: Fabio Utzig <utzig@apache.org>
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.

2 participants