Skip to content
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

Ignore files in lib directory during build #2351

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

Conversation

metcalf
Copy link

@metcalf metcalf commented Sep 7, 2021

Problem

If the lib directory contains a normal file (in my case a Bazel BUILD file), local compilation with Particle Workbench fails with:

> Executing task: make -f '/Users/andrew/.particle/toolchains/buildscripts/1.10.0/Makefile' compile-user -s <


:::: COMPILING APPLICATION

user hal-dynalib services-dynalib system-dynalib rt-dynalib wiring communication-dynalib platform wiring_globals
cc1: error: /Users/andrew/code/supply_ventilation/lib/BUILD/src: Not a directory
make[3]: *** [../build/target/user/platform-6-m/supply_ventilation/Adafruit_GFX_RK/src/glcdfont.o] Error 1
make[2]: *** [user] Error 2
make[1]: *** [modules/photon/user-part] Error 2
make: *** [compile-user] Error 2
The terminal process "/bin/bash '-c', 'make -f '/Users/andrew/.particle/toolchains/buildscripts/1.10.0/Makefile' compile-user -s'" terminated with exit code: 2.

Solution

Updates the relevant makefile to only consider directories within the lib folder. Derived from: https://stackoverflow.com/questions/13897945/wildcard-to-obtain-list-of-all-directories#comment71990812_13898309

Steps to Test

I'm not aware of build-specific tests, though I assume the tests themselves run the build which would provide some coverage here. You should be able to repro the problem and fix by:

  • Start with a workbench project with v2 libraries.
  • Create a file in lib (e.g. touch lib/foo).
  • Attempt to compile and observe error.
  • Apply this fix, observe successful compilation.

Example App

NA

References

Links to the Community, Docs, Other Issues, etc..


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@avtolstoy
Copy link
Member

@metcalf Thanks for the PR. Can I ask you to sign our CLA?

I've tested a few things manually and so far this does seem to be working as intended. We'll need to run the tests on all platforms (Windows/Linux/Mac) though just in case.

@avtolstoy avtolstoy self-requested a review September 7, 2021 17:25
@metcalf
Copy link
Author

metcalf commented Sep 7, 2021

@avtolstoy: I believe I already signed the CLA. Please tell me if I did something wrong and need to re-do. Thanks!

@avtolstoy
Copy link
Member

@metcalf 👍 I can see it now, thanks!

@laughlinbarker
Copy link

Hi @avtolstoy, I wanted to bump this thread.

I encountered same/similar last week when exploring different options for getting my in-house libraries setup in a test harness. Linking/building my tests was trivial (opted for CMake first), but when trying to build my application, which uses said libraries, the build system was a bit overzealous as it was looking for source files (and failing) in my_lib/CMakeLists.txt/src.

I haven't tested his proposed fix, but will give it a go early next week.

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