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

make: Enable the all-debug build target to be available for all boards #4413

Closed

Conversation

DipSwitch
Copy link
Member

Followup for #4188, make the all-debug target available to all boards so the build targets are consistent on all targets.

@OlegHahm OlegHahm added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Dec 6, 2015
@@ -3,6 +3,10 @@

all:

all-debug: export CFLAGS += -g -O0
Copy link
Member

Choose a reason for hiding this comment

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

I think adding -gdwarf-3 (4 and 5 are not supported by all distributions yet) would be nice to resolve macros to values :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added, if someone doesn't agree we'll find another solution.

Copy link
Member

Choose a reason for hiding this comment

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

I seem to remember that there was a problem with DWARF on iotlab-m3 way back. @thomaseichinger do you remember what it was and if this is still an issue?

@OlegHahm OlegHahm modified the milestones: Release 2015.12, Release 2016.03 Dec 7, 2015
@LudwigKnuepfer LudwigKnuepfer 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, 2016
@LudwigKnuepfer
Copy link
Member

Apparently there is some kind of problem:

$ BOARD=chronos make clean all      
Building application "default" for "chronos" with MCU "cc430".

"make" -C /home/lo/RIOT/boards/chronos
"make" -C /home/lo/RIOT/boards/chronos/drivers
"make" -C /home/lo/RIOT/core
"make" -C /home/lo/RIOT/cpu/cc430
"make" -C /home/lo/RIOT/cpu/cc430/periph
"make" -C /home/lo/RIOT/cpu/msp430-common
"make" -C /home/lo/RIOT/drivers
"make" -C /home/lo/RIOT/drivers/saul
"make" -C /home/lo/RIOT/sys
"make" -C /home/lo/RIOT/sys/auto_init
"make" -C /home/lo/RIOT/sys/auto_init/saul
"make" -C /home/lo/RIOT/sys/fmt
"make" -C /home/lo/RIOT/sys/oneway-malloc
"make" -C /home/lo/RIOT/sys/phydat
"make" -C /home/lo/RIOT/sys/ps
"make" -C /home/lo/RIOT/sys/saul_reg
"make" -C /home/lo/RIOT/sys/shell
"make" -C /home/lo/RIOT/sys/shell/commands
   text    data     bss     dec     hex filename
  14908     106    1042   16056    3eb8 /home/lo/RIOT/examples/default/bin/chronos/default.elf

vs.

$ BOARD=chronos make clean all-debug
Building application "default" for "chronos" with MCU "cc430".

"make" -C /home/lo/RIOT/boards/chronos
"make" -C /home/lo/RIOT/boards/chronos/drivers
"make" -C /home/lo/RIOT/core
"make" -C /home/lo/RIOT/cpu/cc430
/home/lo/RIOT/cpu/cc430/cc430-adc.c: In function 'adc_isr':
/home/lo/RIOT/cpu/cc430/cc430-adc.c:113:48: error: frame allocation destroys caller register due to 'task' [-Werror=attributes]
cc1: all warnings being treated as errors
/home/lo/RIOT/Makefile.base:60: recipe for target '/home/lo/RIOT/examples/default/bin/chronos/cpu/cc430-adc.o' failed
make[2]: *** [/home/lo/RIOT/examples/default/bin/chronos/cpu/cc430-adc.o] Error 1
/home/lo/RIOT/Makefile.base:20: recipe for target 'ALL--/home/lo/RIOT/cpu/cc430' failed
make[1]: *** [ALL--/home/lo/RIOT/cpu/cc430] Error 2
/home/lo/RIOT/examples/default/../../Makefile.include:220: recipe for target 'all' failed
make: *** [all] Error 2

@DipSwitch
Copy link
Member Author

Maybe the dwarf support?

@OlegHahm
Copy link
Member

Hm, nope, it's failing here if I remove the -gdwarf-3, too.

@cgundogan
Copy link
Member

the problem could be that when building with the proposed change the CFLAGS for msp contains -gdwarf-3 and -gdwarf-2 at the same time

@cgundogan
Copy link
Member

it seems to be the combination of -gdwarf-2 and -Os that breaks

@cgundogan
Copy link
Member

let me revoke my statement. It's the O0 that breaks for the cc430

@DipSwitch
Copy link
Member Author

Lol, that you don't see often. No optimalization breaking the code ;)
On 3 Mar 2016 18:12, "Cenk Gündoğan" notifications@github.com wrote:

let me revoke my statement. It's the O0 that breaks for the cc430


Reply to this email directly or view it on GitHub
#4413 (comment).

@DipSwitch DipSwitch added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 14, 2016
@DipSwitch
Copy link
Member Author

Waits for #4954

@OlegHahm
Copy link
Member

Unfortunately I don't think we will find a solution for #4954 today.

@cgundogan
Copy link
Member

Well, the question is whether we should merge anyways? Currently, the cc430 with no optimization is broken on master, not because of this PR

@OlegHahm
Copy link
Member

But this PR makes it much more likely that someone does so.

@cgundogan
Copy link
Member

@DipSwitch can you selectively ignore the O0 for msp430 platforms?

@DipSwitch DipSwitch force-pushed the pr/unify-makefile-target-all-debug branch from 9ea876c to 6657fd9 Compare April 4, 2016 20:51
@DipSwitch DipSwitch closed this Apr 4, 2016
@DipSwitch DipSwitch force-pushed the pr/unify-makefile-target-all-debug branch from 6657fd9 to 05b1968 Compare April 4, 2016 20:54
@DipSwitch DipSwitch reopened this Apr 4, 2016
@DipSwitch
Copy link
Member Author

Ok, something went wrong with the rebase... but something like this?

@DipSwitch DipSwitch removed the State: waiting for other PR State: The PR requires another PR to be merged first label Apr 4, 2016
@miri64 miri64 modified the milestones: Release 2017.01, Release 2016.10 Oct 31, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Jan 11, 2017

Please rebase. Do this make some difference for our CI?

@OlegHahm
Copy link
Member

I think the problem for MSP platforms still exist.

@kYc0o
Copy link
Contributor

kYc0o commented Jan 11, 2017

mmm... I'm thinking about maybe organise the PRs on what's the problem and put them into a github project? Then we can choose a shepherd to put some efforts on solve the common problem on the PRs present on such project and then try to merge them faster... Too much thinking for today xD

@OlegHahm
Copy link
Member

In general this sounds like a good idea - assuming that we have enough "common problems".

@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.01, Release 2017.04 Jan 26, 2017
@kYc0o kYc0o added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 30, 2017
@smlng
Copy link
Member

smlng commented Jan 12, 2018

postponed several times without progress, hence remove milestone.

@smlng smlng removed this from the Release 2018.01 milestone Jan 12, 2018
@smlng smlng removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 12, 2018
@miri64
Copy link
Member

miri64 commented Feb 15, 2018

No significant progress for over a year. Closing as memo for now.

@miri64 miri64 closed this Feb 15, 2018
@miri64 miri64 added the State: archived State: The PR has been archived for possible future re-adaptation label Feb 15, 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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer State: archived State: The PR has been archived for possible future re-adaptation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.