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

Fix unix/Makefile to build on OSX #871

Merged
merged 3 commits into from
Sep 23, 2014
Merged

Fix unix/Makefile to build on OSX #871

merged 3 commits into from
Sep 23, 2014

Conversation

blmorris
Copy link
Contributor

Please comment and review - I am not an expert in the subtleties of Makefiles!

This unix/Makefile change should allow the unix port to build on OSX without breaking stmhal build on OSX or affecting other platforms. Tested build for unix and stmhal on OSX 10.7.5 (Lion):

Force OSX to compile with clang even if gcc is available
Change LDFLAGS syntax to be compatible with clang
Fix questionable syntax on line 90
Remove extraneous tab character on line 102

Force OSX to compile with clang even if gcc is available
Change LDFLAGS syntax to be compatible with clang
Fix questionable syntax on line 90
Remove extraneous tab character
@blmorris blmorris mentioned this pull request Sep 22, 2014
@blmorris
Copy link
Contributor Author

Sounds good, I'll be able to update the PR later this evening.

@blmorris
Copy link
Contributor Author

Actually, your suggestion will change the syntax suggested in #782 (comment) to have clang make the map file.
And I'm also not sure if line 90 should be LDFLAGS or LDFLAGS_MOD

@blmorris
Copy link
Contributor Author

I see what to do now.
Line 32 will be

LDFLAGS = $(LDFLAGS_MOD) -lm -Wl(LDFLAGS_MAP_EXTRA) $(LDFLAGS_EXTRA)

Then LDFLAGS_MAP_EXTRA will be ,-map,$@.map or ,-Map=$@.map$,--cref

Edited to add: That seems to work. Any thoughts on line 90 being LDFLAGS or LDFLAGS_MOD?
Seems to compile fine either way.

@dhylands
Copy link
Contributor

@blmorris Ahh - I missed the -M -m difference. In that case, I would just call it LDFLAGS_MAP and make it equal to -Wl,-map,$@.map or -Wl,-Map=$@.map,--cref

Then you have the option of setting LDFLAGS_MAP to the empty string to not get a map file at all.

@dpgeorge
Copy link
Member

Couldn't the order and map stuff be put in the same variable. Eg:

ifeq ($(UNAME_S),Darwin)
# Force OSX to use clang even if gcc is present, value set in mkenv.mk
# must be ovewritten here to avoid breaking stmhal build on OSX
CC = clang
# Use clang syntax for LDFLAGS
LDFLAGS_ARCH = -Wl,-order_file,$(BUILD)/order.def -Wl,-map=$@.map
else
LDFLAGS_ARCH = -Wl,-Map=$@.map,--cref
endif
LDFLAGS = $(LDFLAGS_ARCH) -lm $(LDFLAGS_EXTRA)

Of course, then you can't easily override to get no map file.

@blmorris
Copy link
Contributor Author

Doesn't LDFLAGS_MOD need to be somewhere in the definition? There are several lines later in the file that manipulate the value of LDFLAGS_MOD, and without a placeholder in LDFLAGS it seems that all of those values simply get skipped over.
As for skipping map file creation, as far as I can tell clang makes the map file without any problems as long as it is given the right syntax, (which is what we are trying to do here), so is there any compelling reason to leave an option to skip it?

@dpgeorge
Copy link
Member

Yes, you are right, LDFLAGS_MOD should be in there:

LDFLAGS = $(LDFLAGS_MOD) $(LDFLAGS_ARCH) -lm $(LDFLAGS_EXTRA)

@blmorris
Copy link
Contributor Author

Got it. I'll test that on my system and update the PR if it works out.

@blmorris
Copy link
Contributor Author

I just realized that LDFLAGS_EXTRA is not used anywhere else in unix/Makefile; am I correct that this is just a placeholder for flags to be added later in the build process?

@dpgeorge
Copy link
Member

am I correct that this is just a placeholder for flags to be added later in the build process?

Yes, it's used if you want to add options from the command line.

Add more specific comments describing what is going on.
@blmorris
Copy link
Contributor Author

Both unix and stmhal build on my system, and it checks out on Travis too. Anything else we might have missed?
Actually Travis is still working...

dpgeorge added a commit that referenced this pull request Sep 23, 2014
Fix unix/Makefile to build on OSX
@dpgeorge dpgeorge merged commit 9f53275 into micropython:master Sep 23, 2014
@dpgeorge
Copy link
Member

Thank you!

@blmorris blmorris deleted the osx_build_fix branch September 23, 2014 14:02
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request May 25, 2018
fcuzzocrea pushed a commit to fcuzzocrea/micropython that referenced this pull request Nov 16, 2023
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.

None yet

3 participants