tobozo / M5Stack-SD-Updater

💾 Customizable menu system for M5Stack, M5Unified and ESP32-Chimera-Core - loads apps from the Micro SD card. Easily add you own apps
MIT License
310 stars 42 forks source link

Specify library dependencies in library.properties #123

Closed per1234 closed 4 years ago

per1234 commented 4 years ago

Specifying the library dependencies in the depends field of library.properties causes the Arduino Library Manager (Arduino IDE 1.8.10 and newer) to offer to install any missing dependencies during installation of this library.

arduino-cli lib install will automatically install the dependencies (arduino-cli 0.7.0 and newer).

Reference: https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification#libraryproperties-file-format

NOTE: I did not add ArduinoJson and M5StackSAM as dependencies because these are only dependencies of the example sketches, not the library itself. However, I also think it would be a reasonable decision to install dependencies of the examples as well. If you would prefer these libraries be added as dependencies, I'm happy to update this PR accordingly.

tobozo commented 4 years ago

hey, thanks for posting this :+1:

I'll add M5Stack-Core as a depends entry for the next release, but this SD-Updater library can depend either on M5Stack core or ESP32-Chimera-Core.

The ESP32-Chimera-Core hasn't been submitted (yet) to the Arduino library listing because it shares the same namespace (it's an improved clone of the M5Stack-Core) and I don't fully control how the IDE will chose either core at compilation time.

I could change the namespace but there would be no point in maintaining this library.

I could also use a superior version number in library.properties butI don't want to predate the M5Stack Core library.

Should I keep this library unlisted by the Arduino Library Manager or is there a clean way to get those to coexist ?

If so, does the "depends" entry in the library.properties handle ambiguous dependencies, or could it point to a repository not listed by the Arduino library manager ?

per1234 commented 4 years ago

does the "depends" entry in the library.properties handle ambiguous dependencies

Each library added to the Arduino Library Manager index must have a unique name value in library.properties. It is that name value that the depends field refers to. So in order for you add ESP32-Chimera-Core to the Library Manager index, you would need to change the name value to something other than M5Stack (since that is already taken by the m5stack/M5Stack library). Then the M5Stack-SD-Updater library's depends field could be set to the unique library name you chose.

Now this doesn't address the issue of which of the two libraries would be selected to match an #include <M5Stack.h> if the user happened to have both m5stack/M5Stack and tobozo/ESP32-Chimera-Core installed. In fact, I expect it would be m5stack/M5Stack because the folder/filename match is one of the factors used to determine this. You could force tobozo/ESP32-Chimera-Core by adding a unique header filename to that library and then making sure the user adds an #include directive for that filename before any ambiguous #include directives.

could it point to a repository not listed by the Arduino library manager

No.

tobozo commented 4 years ago

thanks for confirming this, I'll publish the ESP32-Chimera-Core library under a different namespace and stick with the non ambiguous choice of "manually deleting the other library" like I already do with various custom clones of Adafruit modules.

I've updated this repository as per your recommendations and I'm planning to produce a new release soon.

Is there a tool (preferably a travis script) I could add to automatically check for compliance before I produce this release ?

per1234 commented 4 years ago

Is there a tool (preferably a travis script) I could add to automatically check for compliance before I produce this release ?

By "compliance", do you mean compliance with the requirements of the Arduino Library Manager indexer? If so, you asked the right person!

My CI script has a function check_library_manager_compliance that does just that: https://github.com/per1234/arduino-ci-script#check_library_manager_compliance-librarypath

That is intended to be used in a Travis CI build to ensure that compliance with the requirements of the Library Manager indexer weren't inadvertently broken during development.

I also have a script I run on all the incoming Library Manager inclusion requests: https://github.com/per1234/ino-library-manager-validator that runs check_library_manager_compliance, but also checks whether the library has a unique name value. The latter check would not be useful once the library has been added to the Library Manager index, so I haven't added it to the check_library_manager_compliance function.

tobozo commented 4 years ago

All right, after fiddling around I came up with this travis script

I've commented out the install_package() function because it didn't seem to work and I couldn't understand why.

per1234 commented 4 years ago

I'm sorry about the problem with the install_package function. I discovered it had been broken by some new output from the Arduino IDE's CLI that was added in Arduino IDE 1.8.10. I was very sick for the last half of 2019, so haven't kept up with my volunteer development projects. In addition, Travis CI killed my monthly cron jobs for some reason, so the CI build of arduino-ci-script had never been ran since Arduino IDE 1.8.10 was released.

I've now fixed the bug in arduino-cli-script and created a new release. Your CI build is working with the install_package function restored: https://travis-ci.org/per1234/M5Stack-SD-Updater/builds/634739381

tobozo commented 4 years ago

thanks ! will try that right away (and probably go on fixing those warnings)

so the next step would be to call the ino-library-manager-validator when a PR is made onto the master, and create a new release/tag if the tests are successful, right ?

[edit]

results looking good, I'm using this before calling all the formatting checks, this prevents unwanted recursion from messing up the results.

  - git submodule status | rm -Rf `cut -d ' ' -f 3`

I also added this filter -type f \( ! -iname ".gitmodules" \) to the tabs formatting check because the .gitmodules file seems to have tabs and there's nothing I can do about it

image

per1234 commented 4 years ago

Nice! I'm glad to see arduino-ci-script and those formatting checks being of use to other people. I have found them to be very helpful in my own projects and hoped that by publishing them I could provide an easy path for others to add CI builds to their projects.

the next step would be to call the ino-library-manager-validator when a PR is made onto the master, and create a new release/tag if the tests are successful, right

I think you already figured it out, but just to be sure: ino-library-manager-validator is only intended to be used to make sure the library meets the requirements for being added to the Library Manager index. The idea is that if the script passes, then you can be certain that when cmaglie goes to add your library to the Library Manager index, there won't be any problems that block that addition. In the past, it was common for a problem to pop up when cmaglie went to add it, which would mean that the library would not be added on that round. The submitter would then need to fix the problem and wait for the next round of LM additions. That could sometimes mean a total wait of 3 weeks or more between the initial request and the library finally being in Library Manager. So I started running ino-library-manager-validator on all libraries as soon as the requests come in to the arduino/Arduino issue tracker, and letting them know if the script detects any problems. That way, the submitter can have time to fix the problem and have the library fully compliant by the time cmaglie comes through to add all the outstanding requests.

ino-library-manager-validator is not very useful for CI because it checks out the newest tag (which is what the Library Manager indexer job does), whereas for CI you want to test the tip of the branch. The duplicate name check is also not useful once the library is in the LM index because the library name will match against its own entry in the index and fail the check.

The parts of ino-library-manager-validator that are useful for CI are the checks done by arduino-ci-script's check_library_manager_compliance function, which I see you are already using.