-
Notifications
You must be signed in to change notification settings - Fork 176
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
Skip mbed-os import in subdirs #474
base: master
Are you sure you want to change the base?
Conversation
NOTE: this can be generalized by keeping track of all the repos imported at the cwd_root level. A solution for this could be to keep a global set that holds the names of all the *.lib files that have been used to import so far. This would still be isolated to Repo.getlibs. Alternatively, one could use a similar global set, but go a little deeper: save the actual URLs that have been used. This would be managed in Repo.fromurl. This would be a little awkward, however, in that Repo.fromurl would return None when a redundant import is found. I assume we'd want some action() call stating why something was skipped. |
@wdwalker firstly thanks for the PR. The implementation as it is creates ambiguity about what version will be used in case of conflicting dependencies. As an example a library may have mbed-os.lib at rev #ABC, but the program may have mbed-os.lib at rev #XYZ. The user end will end up with mbed-os#XYZ if they import the program, which is different if they import the library (which would give them #ABC otherwise). Also we'd like to encourage a clean development model where a library is NOT the root of the tree. Here's some reasoning behind it:
Do you feel that a development tree like below would work for you?
|
@screamerbg - many thanks for the response. I think the development tree looks fine and it is what I've been working with in my own sandbox. What I'm looking to do is have my library contain its own TESTS and such; those need mbed-os headers and sources to compile against. This will more easily enable library development independently of an app and automated CI. I definitely agree about the ambiguity issue and also agree that the app should dictate the mbed-os version. One might argue that knowing which version of the mbed-os sources the library was developed against is useful info, though. Some sort of compatibility framework might be nice (though out of scope for this PR). Today, I do an 'mbed add mbed-os' whenever I work with my library directly and I also do the same in the library's .travis.yml. I was hoping to be able to avoid this via the patch proposed in this PR, but I'm OK with continuing to do what I do today. Happy to close this PR if you think it's a dead end. Please let me know. |
@wdwalker I think we should rather show a good visible warning when duplicate clones are detected. This will help the user understand that there might be underlying problem and duplicate code. |
The change is rather straightforward - if we come across an mbed-os.lib file in a library imported by an app, skip it. My test set up is as follows:
mbed import of both yields the desired results:
My knowledge of mbed-cli.py is limited, however, so the above may not be sufficient. Please review - I will not be at all offended by rejection.
Fixes #472 - mbed-cli can clone mbed-os.lib more than once.