Skip to content
This repository was archived by the owner on May 19, 2022. It is now read-only.

Improve build#96

Merged
DanielOaks merged 10 commits intosekaiproject:masterfrom
TellowKrinkle:FixMacOSVersion
Mar 10, 2020
Merged

Improve build#96
DanielOaks merged 10 commits intosekaiproject:masterfrom
TellowKrinkle:FixMacOSVersion

Conversation

@TellowKrinkle
Copy link
Copy Markdown
Contributor

@TellowKrinkle TellowKrinkle commented Mar 6, 2020

  • Use ./autogen.sh to generate SDL2_image's configure file in build
    • Previously the build would fail because this was missing
  • Compile SMPEG with -std=C++03 because it errors in C++11
  • Properly compile the final executable with the right macOS deployment target
    • Previously all the libs were compiled with -mmacosx-version-min=10.5 but the final binary wasn't, making it all pointless
  • Increase the macOS deployment target to 10.7
    • Xcode 10 and up no longer allow compiling for macOS versions that need libstdc++
    • If you're using Xcode 9 or below, you can compile for 10.5 with make MACOSX_DEPLOYMENT_TARGET=10.5
  • Add enough dependencies to the makefile that you can now compile with -j8 without it breaking
  • Fix the issue where internal lib builds would pickup homebrew SDL
    • Was due to them finding it with pkg-config. Normally you'd fix this by setting a PKG_CONFIG_PATH but the SDL pkg-config file is also missing the static library list so it's better to pretend pkg-config doesn't exist at all

Comment on lines +64 to +68
dnl The script previously overwrote CXXFLAGS with CFLAGS, but that's not always okay
dnl Save CFLAGS here, then clear it, and finally use it to append to both
save_CFLAGS="$CFLAGS"
CFLAGS=""

Copy link
Copy Markdown
Contributor

@DanielOaks DanielOaks Mar 6, 2020

Choose a reason for hiding this comment

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

Just curious, is this also upstream or when we update smpeg will we need to re-add these changes on top again? (or do newer versions of smpeg just fix this for us already, aha)

Comment thread src/extlib/src/smpeg-2.0.0/configure.in Outdated
Comment on lines +220 to +222
CXXFLAGS="$CXXFLAGS -std=c++03 $CFLAGS"
dnl Restore pre-save CFLAGS (see above comment on creation of save_CFLAGS)
CFLAGS="$save_CFLAGS $CFLAGS"
Copy link
Copy Markdown
Contributor

@DanielOaks DanielOaks Mar 6, 2020

Choose a reason for hiding this comment

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

same question as above~

@DanielOaks
Copy link
Copy Markdown
Contributor

DanielOaks commented Mar 6, 2020

Nice, this is super cool! Honestly I can't really dig in and review this properly, but given the age of this code and all I'd be pretty happy to merge this in once we answer that smpeg question. Thanks very much for the work mate :D

@TellowKrinkle
Copy link
Copy Markdown
Contributor Author

Ahh no that's not upstream I should probably look into trying to upstream it

@TellowKrinkle
Copy link
Copy Markdown
Contributor Author

PR'd to upstream (or whatever the email equivalent of a PR is)
So as long as that gets merged you shouldn't have to worry if you update smpeg

@DanielOaks
Copy link
Copy Markdown
Contributor

Awesome, thanks very much mate! We'll see what they do but either way we'll have this PR to refer to. I'm happy to merge this whenever you think it's ready to go :D

@TellowKrinkle
Copy link
Copy Markdown
Contributor Author

Yeah that's the first message in 6 years so I'm not sure if anyone's still there but we'll see (if no one's there, there won't be any smpeg updates to worry about so I guess it wouldn't matter)

And it looks like I broke the static lib linux build with my disabling of pkg-config, let me find another solution to this...

We don't include a webp lib, and otherwise it picks it up from homebrew and tries to link to it
@TellowKrinkle
Copy link
Copy Markdown
Contributor Author

And that one broke my linux steam build
Need to sleep, will try again tomorrow

-std=c++03 makes it unable to find sys/select.h for some reason
@TellowKrinkle
Copy link
Copy Markdown
Contributor Author

Okay seems to be working on Linux with and without steam, macOS, and Windows msys2
So I think I'll call it good

@euank
Copy link
Copy Markdown
Contributor

euank commented Mar 10, 2020

These changes LGTM, thanks @TellowKrinkle!

I'll leave it to @DanielOaks to merge since he took a look already as well.

@DanielOaks
Copy link
Copy Markdown
Contributor

Awesome, thanks again for the build improvements mate! :D

@DanielOaks DanielOaks merged commit bc26c33 into sekaiproject:master Mar 10, 2020
@TellowKrinkle TellowKrinkle deleted the FixMacOSVersion branch March 11, 2020 04:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants