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 Makefile builds #184

Merged
merged 1 commit into from Dec 4, 2015
Merged

Fix Makefile builds #184

merged 1 commit into from Dec 4, 2015

Conversation

meadori
Copy link
Contributor

@meadori meadori commented Dec 4, 2015

This commit fixes two problems exposed by Makefile builds
(-m): (1) the cmark library directory is wrong and (2) the
COMMON_CMAKE_OPTIONS are not quoted when building swift (thus
"Unix Makefiles" ends up as two command line arguments).

Tested by building for Ninja (default), Make (-m), and XCode (-x).

This commit fixes two problems exposed by Makefile builds
(-m): (1) the cmark library directory is wrong and (2) the
COMMON_CMAKE_OPTIONS are not quoted when building swift (thus
"Unix Makefiles" ends up as two command line arguments).

Tested by building for Ninja (default), Make (-m), and XCode (-x).
@jrose-apple
Copy link
Contributor

The Makefile build isn't really a recommended way to build Swift at this point, but that's no reason to get this wrong. Thanks!

jrose-apple added a commit that referenced this pull request Dec 4, 2015
@jrose-apple jrose-apple merged commit 7245c07 into apple:master Dec 4, 2015
@jrose-apple
Copy link
Contributor

Also, thanks for stating which configurations you tested, and for actually testing all three of them. :-)

@meadori
Copy link
Contributor Author

meadori commented Dec 4, 2015

I figured the Makefile build wasn't used much, but I reasoned either it should be fixed or removed. The fix was simple. Maybe it can be deprecated later on down the road.

Thank you for merging!

@thawkins
Copy link

thawkins commented Dec 5, 2015

Makefile build produces far better progress and output information over ninja at the moment, i use it becuase of that. It also seems to impact the machine less, ninja builds seem to consume far far more memory, my 8g machine with 2 threads set, swaps under ninja but not under cmake/makefile.

@jckarter
Copy link
Member

jckarter commented Dec 5, 2015

@thawkins ninja -v gives more make-like detailed output. You might try that.

@thawkins
Copy link

thawkins commented Dec 5, 2015

Off topic, but where do we register issues? Bugs.swift.org requires a login, but has no registration system.

@jckarter
Copy link
Member

jckarter commented Dec 5, 2015

Click "sign up" under the login box.

@thawkins
Copy link

thawkins commented Dec 5, 2015

Ahhh, sused it, the version that is being shown to things it thinks are
mobile is not showing that link, its only visable on desktop machines, i
normaly do all my web browsing on a 12 inch android tablet, so i was not
seeing the link.

On Sat, Dec 5, 2015, 13:49 Joe Groff notifications@github.com wrote:

Click "sign up" under the login box.


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

@gparker42
Copy link
Contributor

Please file a bug report about the, er, bug reporter.

@liangdzou
Copy link

There is a bug report (SR-65) for the second problem.

dabelknap pushed a commit to dabelknap/swift that referenced this pull request Jan 16, 2019
maldahleh pushed a commit to maldahleh/swift that referenced this pull request Oct 26, 2020
Update README to Reflect Recent API changes
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
…-patch-1

Updates Sourcery hash for conformance changes
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

6 participants