Skip to content

examples/pkgs: rename modules with duplicate names#7951

Merged
kaspar030 merged 2 commits intoRIOT-OS:masterfrom
cladmi:pr/conflicting_module_names
Nov 13, 2017
Merged

examples/pkgs: rename modules with duplicate names#7951
kaspar030 merged 2 commits intoRIOT-OS:masterfrom
cladmi:pr/conflicting_module_names

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Nov 7, 2017

Some examples and packages root directory use a module name that is already defined somewhere else.

Building works right now because when both an APPLICATION an a MODULE have the same
name AR just adds objects for both to the same library MODULE.a.

It currently works because they do not have conflicting file names.

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 7, 2017

I found them with #7952 but only tested native and samr21-xpro.

@cladmi cladmi added the Area: build system Area: Build system label Nov 7, 2017
@kaspar030
Copy link
Copy Markdown
Contributor

I don't understand the second commit. Are you renaming the package modules to pkg-X?

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 10, 2017

I am renaming the package root module to pkg-X yes.

For libfixmath for example: both the package root and the libfixmath directory Makefile have MODULE=libfixmath. For the package root its because the directory is named libfixmath.
So they both define the libfixmath module and both produce the libfixmath.a archive.

Currently everything is working because ar just adds new files to the archive and does not care that two modules modify it.

What I did, is rename the root directory module to some dummy name. It will produce an empty unused archive but not touch the real libfixmath.a one.

@kaspar030
Copy link
Copy Markdown
Contributor

I am renaming the package root module to pkg-X yes.

Where do they get selected for linking?

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 13, 2017
@cladmi cladmi force-pushed the pr/conflicting_module_names branch from 83ee337 to 21dfcc9 Compare November 13, 2017 14:31
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 13, 2017

They don't get selected for linking but they are empty libraries.

No source files in the module Makefile directory:

RIOT/tests/libfixmath$ ls bin/pkg/native/libfixmath/
0001-Move-to-RIOT-Makefiles.patch                                benchmarks  images      unittests
0002-Fix-warnings.patch                                          contrib     libfixmath
0003-Adapt-unittests-for-RIOT.patch                              fixsingen   Makefile
0004-Change-conflicting-module-name-for-pkg-root-director.patch  fixtest     README.md

And the produced library is indeed empty

RIOT/tests/libfixmath$ ar t bin/native/pkg-libfixmath.a
RIOT/tests/libfixmath$ 

@kaspar030
Copy link
Copy Markdown
Contributor

They don't get selected for linking but they are empty libraries.

I see. Seems to me the package Makefile is more complicated than needed. That package's Makefile.include could just add the necessary folders to "DIRS", saving the empty module.

Anyhow, thanks for the explanation!

Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 left a comment

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 f89ecfb into RIOT-OS:master Nov 13, 2017
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Nov 14, 2017

I also think that, in general, setting DIRS should be handled by the main and sub Makefile.include and not submodules Makefile but that is a separate task.

@cladmi cladmi deleted the pr/conflicting_module_names branch November 16, 2017 10:39
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants