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

[WIP] Git2 framework #570

Closed
wants to merge 38 commits into from
Closed

[WIP] Git2 framework #570

wants to merge 38 commits into from

Conversation

phatblat
Copy link
Member

@phatblat phatblat commented Apr 1, 2016

Migrating phatblat#1 into the main project. Also collapsed separate https://github.com/phatblat/git2 repo into this one, but kept the git2 Xcode project separate as a subproject. This way, git2 takes care of building the OpenSSL and libssh2 static libraries, which are only used by libgit2 and the output is a git2.framework that sits alongside ObjectiveGit.framework.

The real reason for this change is to treat git2 as a real module instead of a submodule (ObjectiveGit.git2) finally resolving those pesky module issues since the libgit2 #imports are expecting "git2" at the top level.

Addresses #436, #523, #542

I'm sure there's some cleanup necessary before this is ready to merge, but it should be good enough for review in this initial state.

@pietbrauer
Copy link
Member

What I have been wondering about is why we can't just compile libgit1 with modules enabled. It fails to build. I think this would be the right way to do it?

@phatblat
Copy link
Member Author

phatblat commented Apr 1, 2016

Building libgit2 as it's own module is certainly possible, but consuming projects would need to copy the library and headers to the correct locations for it to work. The nice thing about a framework is that it's self contained.

# Conflicts:
#	External/libgit2
@alehed
Copy link
Contributor

alehed commented Jun 28, 2016

Any chance this can already be used for production work?

@alehed
Copy link
Contributor

alehed commented Jun 29, 2016

I tried it out and it seems to work. The problem was that it kept complaining about inttypes.h not being modular in common.h. Commenting it out and adding the missing dependency directly to config.c seemed to do the trick. Not quite sure what's best to do on the long run though.

@phatblat
Copy link
Member Author

phatblat commented Jul 6, 2016

@alehed Yes, this branch with new Xcode build for libgit2 works, I've been playing around with it in a hobby app for months. Fixing those module errors coming from inttypes.h was one of the goals of this change, and as you can see it doesn't resolve it. 😞

@pietbrauer
Copy link
Member

What's the state of this noting that we get the same non modular header error again, even with the framework?

@phatblat
Copy link
Member Author

I have mixed feelings about this. I'm disappointed that this change doesn't solve one of the main reasons for it. However, I do think that an Xcode-based build will be easier to maintain than a cmake wrapper script. Also, the build is slightly faster since each architecture can build in parallel.

@tiennou
Copy link
Contributor

tiennou commented Jul 18, 2016

Can you guys give a spin to libgit2/libgit2#3781, see how it works for you ?

IIRC I got to the point where I had a linkable git2.framework, but wasn't able to package the various dependencies so I don't think it would work as a module import.

@alehed
Copy link
Contributor

alehed commented Feb 19, 2017

I'm currently trying to update it for master (see alehed/git2_framework_v251), but I'm having trouble with pthread (it's giving me the "implicit declaration" and "undefined symbol" treatment). The inttypes problem can probably be taken care of upstream.

@tiennou personally, I think building from source using it's own module is preferable over linking it with a pre-packaged framework (at least for development), but that is just my opinion. Manually updating it for every libgit2 release is a pain but I don't see an alternative.

@phatblat did you encounter anything like that when you created git2.xcodeproj?

@tiennou
Copy link
Contributor

tiennou commented Feb 19, 2017

The alternative I was proposing was to make the CMake Xcode generator work correctly in libgit2 itself (which would take care of the pain of manual updates), so I don't understand your point about pre-packaged. What I was missing is a way to make CMake copy the (built) dependencies (openssl & friends) at the correct place in the generated libgit2, so the resulting framework would be self-sufficient.

@alehed
Copy link
Contributor

alehed commented Feb 19, 2017

What I don't like about cmake generated Xcode projects is that they work using extra scripting hooks that rely on cmake and they also have a weird target system. If there was a way to generate a clean Xcode project that is contained in itself, I would go for that.

It is possible to copy around files using the cmake syntax (using the configure_file command, but I don't remember exactly how to do it).

@tiennou
Copy link
Contributor

tiennou commented Feb 20, 2017

Sorry, my memory is too hazy to remember what you're defining as "weird target system". If you want to investigate, I just rebased my PR against master, so you can give it a spin.

From your libgit2 tree, mkdir build && cd build && cmake -G Xcode -DBUILD_FRAMEWORK=ON ... You'll end up with a libgit2.xcodeproj with a git2 "framework" target. Obviously, this is all WIP 😉 .

@alehed
Copy link
Contributor

alehed commented Feb 20, 2017

I should have probably used the wording "excessive targets":

targets

vs.

targets2

The good news is that it builds. Maybe I can use it to fix the manual Xcode project.

@pietbrauer
Copy link
Member

I am closing this as no one has pushed to this PR in a year. Feel free to reopen if still valid.

@pietbrauer pietbrauer closed this Feb 27, 2018
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.

4 participants