allow tag to be used in mbed import

[studavekar]

Fix for IOTCORE-1104

Branching strategy is changing starting 5.12 for example. mbed import currently doesn't allow the user to specify branches/tags. There was a check which explicitly restricting to import tags other than latest or tip maybe there is some limitation.

This change will have minimal impact on user workflow they can continue using the same command

• Example for branch mbed import https://github.com/ARMmbed/mbed-os-example-blinky#mbed-os-5.12 blinky-5-12 -vvv
• Example for tag mbed import https://github.com/ARMmbed/mbed-os-example-blinky#mbed-os-5.12.0-rc2 blinky-5-12-rc2 -vvv

@screamerbg can you please review it.

cc @theotherjimmy @bridadan @SenRamakri

[bridadan]

The change seems fine to me. I'd like to double check this with @screamerbg to ensure there isn't something that's being missed by not having this check.

[bridadan]

Also as note, this affects what mbed add can accept as well. Part of the Mbed CLI goals is to always create a reproducible build/source state. However, since branches (and technically tags as well) can "move", we want to be sure that when you invoke mbed add with a branch or tag that the actual commit is written to the .lib file. This way the existing "reproducible build" functionality is preserved while still enabling the behavior requested in this issue.

We tested this locally and this is the behavior that we saw, so that behavior still seems to be preserved!

[bridadan]

I did a bit more testing, and there is a change that this introduces. This allows existing .lib files in programs to be able to specify branches and tags instead of just commits, which is a change in behavior. I'd say this behavior change is outside the scope of this PR.

The existing behavior with the error was removed in this PR, so perhaps the error can just be moved to when reading .lib files?

[studavekar]

I did a bit more testing, and there is a change that this introduces. This allows existing .lib files in programs to be able to specify branches and tags instead of just commits, which is a change in behavior. I'd say this behavior change is outside the scope of this PR.

Thanks for additional testing 👍. This is a generic change which allows the user to provide tags/branches across. if the user wants to point a .lib to a branch/tag he/she should have the flexibility to do so on the same lines of mbed import and mbed add. This behavior change/ feature addition is in line with the requirement where the user should have the flexibility to provide branches/tags.

I also thought of "reproducible build" part but all the bets are off when we allow mbed import and mbed add to specify branches.

[adbridge]

Example for tag mbed import https://github.com/ARMmbed/mbed-os-example-blinky#mbed-os-5.12.0-rc2 blinky-5-12-rc2 -vvv

What is the blinky-5-12-rc2 here? I would expect to only have to specify one tag ?

[studavekar]

What is the blinky-5-12-rc2 here? I would expect to only have to specify one tag ?

blinky-5-12-rc2 is optional positional arguments specifying the destination name where we want to import the project.

C:\>mbed import --help
usage: mbed import [-h] [-I] [--depth [DEPTH]] [--protocol [PROTOCOL]]
                   [--insecure] [--offline] [--no-requirements] [-v] [-vv]
                   url [path]
Imports mbed program and its dependencies from a source control based URL
(GitHub, Bitbucket, mbed.org) into the current directory or specified
path.
Use "mbed add <URL>" to add a library into an existing program.

positional arguments:
  url                   URL of the program
  path                  Destination name or path. Default: current directory.

[studavekar]

I did a bit more testing, and there is a change that this introduces. This allows existing .lib files in programs to be able to specify branches and tags instead of just commits, which is a change in behavior. I'd say this behavior change is outside the scope of this PR.

The existing behavior with the error was removed in this PR, so perhaps the error can just be moved to when reading .lib files?

The behavior of what .libs can have is not documented alteast https://os.mbed.com/docs/mbed-os/v5.11/tools/working-with-mbed-cli.html or in the help text. Some background can be found in #224 thanks @bridadan for providing the issue.

[studavekar]

@bridadan have made suggested changes can you re-review. if a named branch is added in .lib it will error. Its inline with current behavior have made the error message more verbose than before.

[mbed] Working path "/home/arm/delthis2/mbed-os-example-blinky" (program)
[mbed] ERROR: named branches not allowed in .lib, offending lib is sd-driver.lib

[screamerbg]

I'm happy with the change. Please see small comment about upper case error message.

[screamerbg]

Please see @bridadan's comment about existing libraries/projects.

[bridadan]

Changes look good to me! Thanks for addressing my comments.