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 building with Xcode 13.1 and macOS 12.0 SDK. #2141

Merged
merged 2 commits into from Nov 13, 2021

Conversation

nevack
Copy link
Member

@nevack nevack commented Nov 13, 2021

Fixes #2140

@@ -294,7 +294,7 @@
BE1183780CE161390002D0F3 /* libminiupnp.a in Frameworks */ = {isa = PBXBuildFile; fileRef = BE1183480CE160960002D0F3 /* libminiupnp.a */; };
BE75C38A0C72A1ED00DBEFE0 /* libevent.a in Frameworks */ = {isa = PBXBuildFile; fileRef = BE75C3490C729E9500DBEFE0 /* libevent.a */; };
BEFC1C050C07753500B0BB3C /* libtransmission.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 4D18389709DEC0030047D688 /* libtransmission.a */; };
BEFC1C1A0C07756200B0BB3C /* daemon.c in Sources */ = {isa = PBXBuildFile; fileRef = BEFC1C0E0C07756200B0BB3C /* daemon.c */; };
BEFC1C1A0C07756200B0BB3C /* daemon.cc in Sources */ = {isa = PBXBuildFile; fileRef = BEFC1C0E0C07756200B0BB3C /* daemon.cc */; };
Copy link
Member Author

Choose a reason for hiding this comment

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

Xcode reformatted on save, I think this is lefover after C++ conversion

@@ -520,8 +520,8 @@
4D3EA0A908AE13C600EA10C2 /* IOKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = IOKit.framework; path = System/Library/Frameworks/IOKit.framework; sourceTree = SDKROOT; };
4D8017E810BBC073008A4AF2 /* torrent-magnet.cc */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = "torrent-magnet.cc"; sourceTree = "<group>"; };
4D8017E910BBC073008A4AF2 /* torrent-magnet.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "torrent-magnet.h"; sourceTree = "<group>"; };
4D80185710BBC0B0008A4AF2 /* magnet-metainfo.cc */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = magnet-metainfo.cc; sourceTree = "<group>"; };
4D80185810BBC0B0008A4AF2 /* magnet-metainfo.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = magnet-metainfo.h; sourceTree = "<group>"; };
4D80185710BBC0B0008A4AF2 /* magnet-metainfo.cc */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = "magnet-metainfo.cc"; sourceTree = "<group>"; };
Copy link
Member Author

Choose a reason for hiding this comment

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

Xcode reformatted on save

);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "cd third-party/libnatpmp && rm -f VERSION\n";
Copy link
Member Author

Choose a reason for hiding this comment

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

Just delete VERSION file before this target is built.
One downside - it makes subodule dirty.
But previous build schema makes submodules dirty anyway.

HEADER_SEARCH_PATHS = (
"third-party/libevent/include",
);
HEADER_SEARCH_PATHS = "third-party/libevent/include";
Copy link
Member Author

Choose a reason for hiding this comment

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

Xcode reformatted on save, Project view still shows that it's Xcode-8.0 compatible.

@nevack nevack force-pushed the nevack/macos-fix-xcode-13-build branch from f57d47d to e4f1f95 Compare November 13, 2021 13:57
@@ -2331,7 +2332,7 @@
files = (
);
inputPaths = (
"third-party/miniupnpc/VERSION",
"third-party/miniupnpc/miniupnpc_VERSION",
Copy link
Member Author

Choose a reason for hiding this comment

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

File is renamed in other script step before this step.

@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

@mikedld What do you think? Do you have any ideas how to make it less dirty?

@mikedld
Copy link
Member

mikedld commented Nov 13, 2021

I think adding more logic to Xcode project is counter-productive if we're aiming at getting rid of it. Also, I suspect this doesn't fix CMake build that uses the same SDK. Clearer way would be to remove/rename those files upstream and in our patch branches (in https://github.com/transmission/miniupnpc and https://github.com/transmission/libnatpmp), bumping submodule revisions afterwards.

@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

I think adding more logic to Xcode project is counter-productive if we're aiming at getting rid of it. Also, I suspect this doesn't fix CMake build that uses the same SDK. Clearer way would be to remove/rename those files upstream and in our patch branches (in https://github.com/transmission/miniupnpc and https://github.com/transmission/libnatpmp), bumping submodule revisions afterwards.

I have zero problems with building with CMake, same machine, same Xcode toolchain and macOS SDK.

Yes, I also think that manual Xcode project should be dropped in favor of cmake's Xcode generator.
But before it's done, I would be really happy to be able to edit obj-c code in xcode and build code right there.

Of course, I can make PRs in transmission forks of this libraries and another PR in this repo with bumping submodules revisions, if you think this is better approach.

@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

@mikedld Can you create post-2.2.3-transmission branch in https://github.com/transmission/miniupnpc repo?
I'll make PR with latest miniupnpc tag miniupnpc_2_2_3 with your fixes cherry-picked and VERSION file removed.

@mikedld
Copy link
Member

mikedld commented Nov 13, 2021

The fact that it works for CMake got me thinking. Perhaps the easier fix would be to move those include paths from HEADER_SEARCH_PATHS (seemingly -isystem) to USER_HEADER_SEARCH_PATHS (seemingly -I).

@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

The fact that it works for CMake got me thinking. Perhaps the easier fix would be to move those include paths from HEADER_SEARCH_PATHS (seemingly -isystem) to USER_HEADER_SEARCH_PATHS (seemingly -I).

I'll try that!

@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

The fact that it works for CMake got me thinking. Perhaps the easier fix would be to move those include paths from HEADER_SEARCH_PATHS (seemingly -isystem) to USER_HEADER_SEARCH_PATHS (seemingly -I).

@@ -3193,13 +3196,16 @@
                                CLANG_CXX_LIBRARY = "libc++";
                                CLANG_ENABLE_OBJC_ARC = NO;
                                HEADER_SEARCH_PATHS = (
+                                       "$(inherited)",
+                                       "third-party/libevent/include",
+                                       "third-party/libutp",
+                               );
+                               USER_HEADER_SEARCH_PATHS = (
                                        "$(inherited)",
                                        "third-party/arc4/src",
                                        "third-party/dht",
                                        "third-party/libb64/include",
-                                       "third-party/libevent/include",
                                        "third-party/libnatpmp",
-                                       "third-party/libutp",
                                        "third-party/miniupnpc",
                                );
                                OTHER_CFLAGS = (

I tried to change it like that, but another problem appears

/Users/nevack/Developer/Projects/transmission/libtransmission/upnp.cc:15:10: error: 'miniupnp/miniupnpc.h' file not found with <angled> include; use "quotes" instead
#include <miniupnp/miniupnpc.h>
         ^~~~~~~~~~~~~~~~~~~~~~
         "miniupnp/miniupnpc.h"
/Users/nevack/Developer/Projects/transmission/libtransmission/upnp.cc:16:10: error: 'miniupnp/upnpcommands.h' file not found with <angled> include; use "quotes" instead
#include <miniupnp/upnpcommands.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~
         "miniupnp/upnpcommands.h"

libtransmission/ is common code, can't change it without considerations.

@nevack nevack force-pushed the nevack/macos-fix-xcode-13-build branch from e4f1f95 to 07037c1 Compare November 13, 2021 15:08
@nevack
Copy link
Member Author

nevack commented Nov 13, 2021

@mikedld Fixed.

Moved third-party/libnatpmp and third-party/miniupnpc (submodules with VERSION file in root), and moved needed headers (upnpcommands.h and miniupnpc.h in miniupnp target) from Project group to Public.

@mikedld mikedld merged commit b573d0c into transmission:master Nov 13, 2021
@nevack nevack deleted the nevack/macos-fix-xcode-13-build branch November 13, 2021 19:57
@ckerr ckerr added os:mac type:build Changes that affect the build system labels Nov 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os:mac type:build Changes that affect the build system
Development

Successfully merging this pull request may close these issues.

[macOS] Build errors with Xcode 13.1
3 participants