Skip to content

restructure nlr.h for undefined archtectures #2216

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

Merged
merged 1 commit into from
Oct 14, 2019
Merged

restructure nlr.h for undefined archtectures #2216

merged 1 commit into from
Oct 14, 2019

Conversation

jerryneedell
Copy link
Collaborator

@jerryneedell jerryneedell commented Oct 14, 2019

This PR is by no means critical - there may be better ways to do this
Background
I was attempting to build CP on a Raspberry Pi with Ubuntu 19.04 installed - just to see if it worked...
When I tried to build mpy-cross - it failed with several instances of this error

make: Entering directory '/home/ubuntu/circuitpython/mpy-cross'
Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity.
GEN build/genhdr/moduledefs.h
In file included from ../py/mpstate.h:34,
                 from ../py/runtime.h:29,
                 from ../py/gc.c:32:
../py/nlr.h:63: error: "MICROPY_NLR_SETJMP" redefined [-Werror]
     #define MICROPY_NLR_SETJMP (1)
 
../py/nlr.h:39: note: this is the location of the previous definition
 #define MICROPY_NLR_SETJMP (0)

The Ubuntu distribution on RPi uses the architecture "aarch64" which is not recognized by nlr.h. This should not necessarily be a problem, but as written nlr.h attempts to redefine MICROPY_NLR_SETJMP if the architecture is not recognized.
This PR just restructures the tests so it is only defined once.

With this change, the build of mpy-cross succeeds.
This change has no impact on the BSP builds other than providing a working mpy-cross.

After all taht, it is not clear there is a real need to be able to build under Ubuntu on an RPi since the build tools work fine under Raspian Buster.

The main reason for this change is to fix a potential error when unrecognized architectures are used. nlr.h should not try to redefine MICROPY_NLR_SETJMP.

I think this is a "harmless" change to avoid that.

Note: I also did a little "clean up" of the indentation to make the section more readable.

@jerryneedell jerryneedell requested a review from dhalbert October 14, 2019 11:29
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @jerryneedell

@tannewt tannewt merged commit 5971794 into adafruit:master Oct 14, 2019
@jerryneedell jerryneedell deleted the jerryn_nlr branch October 19, 2019 09:57
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.

2 participants