Skip to content

cpu/atmega_common: do not export LINKFLAGS#10854

Merged
smlng merged 2 commits intoRIOT-OS:masterfrom
cladmi:pr/atmega_common/unexport/linkflags
Jan 24, 2019
Merged

cpu/atmega_common: do not export LINKFLAGS#10854
smlng merged 2 commits intoRIOT-OS:masterfrom
cladmi:pr/atmega_common/unexport/linkflags

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Jan 23, 2019 •
edited
Loading

Contribution description

cpu/atmega_common: do not export LINKFLAGS

This prevent evaluating LINKFLAGS when not needed and when building
in docker so does not produce errors if avr-ld is not installed.

BUILD_IN_DOCKER=1 BOARD=arduino-mega2560  make --no-print-directory -C examples/hello-world/ clean
/srv/ilab-builds/workspace/git/riot_master/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
/bin/sh: 1: avr-ld: not found

It removes the /bin/sh: 1: avr-ld: not found

Testing procedure

On a machine without avr installed, try compiling in docker for any board using atmega_common/Makefile.include and notice the /bin/sh: 1: avr-ld: not found errors.

BUILD_IN_DOCKER=1 BOARD=arduino-mega2560  make --no-print-directory -C examples/hello-world/
/srv/ilab-builds/workspace/git/riot_test/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
/bin/sh: 1: avr-ld: not found
Launching build container using image "riot/riotbuild:latest".
docker run --rm -t -u "$(id -u)" \
    -v '/srv/ilab-builds/workspace/git/riot_test:/data/riotbuild/riotbase' \
    -v '/srv/ilab-builds/workspace/git/riot_test/build:/data/riotbuild/build' \
    -v '/srv/ilab-builds/workspace/git/riot_test/cpu:/data/riotbuild/riotcpu' \
    -v '/srv/ilab-builds/workspace/git/riot_test/boards:/data/riotbuild/riotboard' \
    -v '/srv/ilab-builds/workspace/git/riot_test/makefiles:/data/riotbuild/riotmake' \
    -v '/srv/ilab-builds/workspace/git/riot_test:/data/riotbuild/riotproject' \
    -v '/usr/share/zoneinfo/Europe/Berlin:/etc/localtime:ro' \
    -e 'RIOTBASE=/data/riotbuild/riotbase' \
    -e 'BUILD_DIR=/data/riotbuild/build' \
    -e 'CCACHE_BASEDIR=/data/riotbuild/riotbase' \
    -e 'RIOTCPU=/data/riotbuild/riotcpu' \
    -e 'RIOTBOARD=/data/riotbuild/riotboard' \
    -e 'RIOTMAKE=/data/riotbuild/riotmake' \
    -e 'RIOTPROJECT=/data/riotbuild/riotproject' \
      -v /srv/ilab-builds/workspace/git/RIOT/.git:/srv/ilab-builds/workspace/git/RIOT/.git \
    -e 'BOARD=arduino-mega2560' \
    -w '/data/riotbuild/riotproject/examples/hello-world/' \
    'riot/riotbuild:latest' make  
Building application "hello-world" for "arduino-mega2560" with MCU "atmega2560".

"make" -C /data/riotbuild/riotbase/core
"make" -C /data/riotbuild/riotbase/drivers
"make" -C /data/riotbuild/riotbase/drivers/periph_common
"make" -C /data/riotbuild/riotbase/sys
"make" -C /data/riotbuild/riotbase/sys/auto_init
"make" -C /data/riotbuild/riotbase/sys/isrpipe
"make" -C /data/riotbuild/riotbase/sys/stdio_uart
"make" -C /data/riotbuild/riotbase/sys/tsrb
"make" -C /data/riotbuild/riotboard/arduino-mega2560
"make" -C /data/riotbuild/riotboard/common/arduino-atmega
"make" -C /data/riotbuild/riotboard/common/atmega
"make" -C /data/riotbuild/riotcpu/atmega2560
"make" -C /data/riotbuild/riotcpu/atmega_common
"make" -C /data/riotbuild/riotcpu/atmega_common/avr_libc_extra
"make" -C /data/riotbuild/riotcpu/atmega_common/periph
   text    data     bss     dec     hex filename
   5726     324     969    7019    1b6b /data/riotbuild/riotproject/examples/hello-world/bin/arduino-mega2560/hello-world.elf
/bin/sh: 1: avr-ld: not found

A shorter output is simply calling clean as described in the commit message.
With this PR the avr-ld: not found should not be printed anymore.

Issues/PRs references

Found while testing arduino-mega2560 on a machine without avr.
Depends on #10853
Tracking issue on removing exports and immediate evaluations #10850

cladmi added 2 commits January 23, 2019 18:31
@cladmi
makefiles/vars.inc.mk: do not export LINKFLAGS
be75f73 
It is only used by `Makefile.include` and should not be used by sub-make
instances.

This is required to prevent evaluating `LINKFLAGS` when not needed and a
required step to not evaluate it on the host with for example `avr-ld`
when building in docker.

I checked usage with:

    git grep -e '(LINKFLAGS)' -e '{LINKFLAGS}'

Packages may be using it but `LINKFLAGS` is a RIOT way of naming
things and even if `generate-xcompile-toolchain` uses `LINK` it does not
use `LINKFLAGS`.
@cladmi
cpu/atmega_common: do not export LINKFLAGS
9cd50a6 
This prevent evaluating `LINKFLAGS` when not needed and when building
in docker so does not produce errors if `avr-ld` is not installed.

```
BUILD_IN_DOCKER=1 BOARD=arduino-mega2560  make --no-print-directory -C examples/hello-world/ clean
/srv/ilab-builds/workspace/git/riot_master/makefiles/toolchain/gnu.inc.mk:18: objcopy not found. Hex file will not be created.
/bin/sh: 1: avr-ld: not found
```

It removes the `/bin/sh: 1: avr-ld: not found`
@cladmi cladmi added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Jan 23, 2019
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 23, 2019
aabadie
aabadie approved these changes Jan 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK

I confirm this fixes the bug when building in docker and no avr-ld is installed on the host. With avr-ld available and using a build from the host, it still works.

@aabadie aabadie added this to the Release 2019.01 milestone Jan 23, 2019
smlng
smlng approved these changes Jan 24, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

tested ACK

@smlng smlng added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jan 24, 2019
@smlng smlng merged commit 50516cb into RIOT-OS:master Jan 24, 2019
@cladmi cladmi deleted the pr/atmega_common/unexport/linkflags branch January 25, 2019 16:42
@cladmi cladmi mentioned this pull request Jan 25, 2019
Closed
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie approved these changes

+1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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 Platform: AVR Platform: This PR/issue effects AVR-based platforms Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

3 participants

@cladmi @aabadie @smlng