Skip to content

Makefile.include: fix missing target for libraries#8911

Merged
kaspar030 merged 1 commit into
RIOT-OS:masterfrom
cladmi:pr/fix/makefile/include/unittests
Apr 17, 2018
Merged

Makefile.include: fix missing target for libraries#8911
kaspar030 merged 1 commit into
RIOT-OS:masterfrom
cladmi:pr/fix/makefile/include/unittests

Conversation

@cladmi

@cladmi cladmi commented Apr 10, 2018

Copy link
Copy Markdown
Contributor

Unittests add libraries in 'BASELIBS' which do not have any rules to be built as
they are built by 'application.inc.mk' and the DIRS variable.
So make complains about missing target for the unittests archives.

The fix tells these files are generated when building '$(APPLICATION_MODULE).a'.

The bug was introduced by #8844

Fixes #8910

@cladmi cladmi force-pushed the pr/fix/makefile/include/unittests branch from 8e3f7ba to 049d4c2 Compare April 10, 2018 14:35
@cladmi cladmi changed the title Makefile.inlude: fix missing target for libraries Makefile.include: fix missing target for libraries Apr 10, 2018
@cladmi

cladmi commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

I will try a PR to show that murdock fails to detect it.

@bergzand

Copy link
Copy Markdown
Member

This fixes #8910 for me. I'm not familiar enough with the makefiles to review the code here though.

@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system labels Apr 10, 2018
@cladmi cladmi added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 10, 2018
@cladmi cladmi requested a review from kaspar030 April 10, 2018 15:27
@cladmi

cladmi commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

Even when doing make clean; make all it works, the commits for murdock will be removed before merging.

@emmanuelsearch can you test ?

@emmanuelsearch

Copy link
Copy Markdown
Member

make clean does help indeed.

@cladmi

cladmi commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

The _SUBMAKE_LIBS is at that point not really defined to the files built by the application makefile because $(BASELIBS) does not have its final value.
But as the $(ELFFILE) rule also uses the same value, the fix works.

I added an issue to track this problem of use before define. #8913

@cladmi cladmi added this to the Release 2018.04 milestone Apr 12, 2018
@cladmi

cladmi commented Apr 12, 2018

Copy link
Copy Markdown
Contributor Author

Fixed the rule because some modules are built by packages makefile.

@kaspar030

Copy link
Copy Markdown
Contributor

please squash!

Unittests add libraries in 'BASELIBS' which do not have any rules to be built as
they are built by 'application.inc.mk', packages and the DIRS variable.
So make complains about missing target for the unittests archives.

The fix tells these files are generated when building '$(APPLICATION_MODULE).a'.

The bug was introduced by RIOT-OS#8844

Fixes RIOT-OS#8910
@cladmi cladmi force-pushed the pr/fix/makefile/include/unittests branch from ad19d71 to 94214cd Compare April 16, 2018 10:55
@cladmi

cladmi commented Apr 16, 2018

Copy link
Copy Markdown
Contributor Author

Rebased and squashed message

@kaspar030 kaspar030 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK.

@kaspar030 kaspar030 merged commit 32807bd into RIOT-OS:master Apr 17, 2018
@cladmi cladmi deleted the pr/fix/makefile/include/unittests branch April 17, 2018 15:22
maxvankessel pushed a commit to maxvankessel/RIOT that referenced this pull request Apr 20, 2018
…nittests

Makefile.include: fix missing target for libraries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tests: Unittests don't build

4 participants