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

Revert "Add OO_WARNING_FLAGS to the projects" #2

Merged
merged 1 commit into from
Aug 12, 2020

Conversation

AnotherCommander
Copy link
Member

Reverts #1
Travis build is failing with
:1:9: error: macro is not used [-Werror,-Wunused-macros]
#define OOCOLLECTIONEXTRACTORS_SIMPLE 1
Full error log at https://travis-ci.org/github/OoliteProject/oolite/builds/717161302

@AnotherCommander AnotherCommander merged commit 15fd35b into master Aug 12, 2020
@AnotherCommander
Copy link
Member Author

AnotherCommander commented Aug 12, 2020

Even after the revert, the build on Travis is failing. I have switched everything on master to the last known Mac-specific version that builds. We will need further investigation in order to understand what is going on and, once we are happy that Travis can build, we can merge. The full build log for the failed attempt is here.

@MaddTheSane : Please try to provide a fixed version of your original PR. This must be based on revision 90eb952 of Mac-specific. If this is not possible, then I think we have arrived at the point where we are hitting a wall with the Mac.

@MaddTheSane
Copy link
Contributor

I realized a little later that xcconfig in the main project also needs to be updated.

So merging OoliteProject/oolite#361 as well as pull request #1 should build.

@AnotherCommander
Copy link
Member Author

Accepting #1 implies acceptance of all changes made by Jens in PR #318 of the Oolite repo (which are currently already in Mac-specific but we have not adopted them yet). As per discussion there, if we do this we will not be able to build releases ourselves. This raises the question: @MaddTheSane are you willing to take over the Mac port maintenance? If we accept your update, apart from not being able to make releases, I expect that the Travis build will be wrecked too because it is set to run on Xcode 7.3. So, before making such a change, we will need to ensure that:

  • Travis continues to build. Changes to travis.yml will most likely be required but I am not familiar with it at all. Someone else will have to look at this.
  • We will still be able to make releases for Mac somehow. If you take over the Mac port, you will have to generate the appropriate Mac binaries at release time since we will not have that option anymore. You will probably need to provide support for the build and bug fixes when required as well. Are you OK with that?
  • All this happens without any kind of side effect on the other two platforms. This work will have to be Mac-exclusive and completely transparent for the Windows and Linux ports.

Once we have an acceptable resolution on the above, we can proceed with the merging. It is likely that you may have to resubmit your PR - I am not sure if and how it can be unreverted if we decide to move ahead with it and I had moments of pure panic yesterday before returning everything to a working condition. If possible I would prefer to avoid further git-related complications.

@MaddTheSane
Copy link
Contributor

I must decline the offer to be a Mac maintainer, sorry.

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