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

Hunterize v3.0.0 #3

Merged
merged 991 commits into from
Aug 9, 2016
Merged

Hunterize v3.0.0 #3

merged 991 commits into from
Aug 9, 2016

Conversation

Osse
Copy link

@Osse Osse commented Aug 2, 2016

This ended up being a huge PR so I don't think this is the best way of doing it. But in any case in my fork you will find a tag named v3.0.0-hunter-1. I created this by looking at the changes between v3.0.0-beta-2 and v3.0.0-beta-2-hunter-2 and making the equivalent changes on top of the v3.0.0 tag.

haberman and others added 30 commits May 18, 2016 13:14
- Add the folder CocoaPods will add to the root folder.
- Move and expand the entries in the objectivec directory.
Ruby oneofs: return default instead of nil for unset fields.
- Add an Xcode 6.3 created default iOS Project.
- Add an Xcode 6.3 created default OS X Project.
- Add Podfiles to for both that use Protobufs from within the tree.
- Add a script to run the tests (and cleanup) to help confirm the state of the
  Protobuf.podspec and sources.
OS X gitignore cleanup and cocoapods integration tests
Otherwise the projects have to be opened once to create user schemes for the
command line builds to work.
Add shared schemes for the CocoaPods integration tests
… variables (PACKAGE_VERSION_EXACT, PACKAGE_VERSION_COMPATIBLE, PACKAGE_VERSION_UNSUITABLE)
- Env solution doesn't seem to always work, use template pod files and copy
  them in place instead.
- Flush the pods cache before and after runs.
- Make pod install verbose to have the info incase something goes wrong.
Make cmake configuration file install path configurable
Make protobuf-config-version.cmake.in set the required variables
…n_followup

Make the CocoaPods integration tests more robust
…mespace

Add protobuf:: namespace to installed targets
added missing closing bracket for _cmakedir_desc in cmake/install.cmake:88
- Add generator constant for the default framework name.
- Add generator api for making the CPP symbol from the name.
- Add generator api to see if it is a bundled proto file.
- Output a CPP conditional and two imports for the core library headers.
- Add helper for generating the #import for file headers to deal with the
  framework imports.
- Add a reference from the unittests to a WKT to use that to inspect how
  imports generate.
- Update the podspec to define the CPP symbol and require pods 1.0 (or later).

Fixes protocolbuffers#1457
Add js/binary/encoder.js to js_EXTRA_DIST.
Better support for using the proto library from a framework.
- Move the ObjC tests into the list and exclude them on linux, this will change
  where in the order they start, since they are longer, it will have other
  things run in parallel instead of them ending up last and taking the longest.
- Switch to the Xcode 7.3 image.
- Drop the use of xctool and stream line things through the full_mac_build.sh
  script. This means we end up with only one build script instead of two.
- Tweaks to the mac build script:
  - Make iOS Xcode version support explicit
  - Support Debug/Release only building
  - Change the OS X min parallel count to 2 to better deal with VMs.
- Split the travis ios tests into the two Xcode Configurations as the logs are
  choking travis.
Working on protocolbuffers#1599, specifically:
- Turn on more warnings that the Xcode UI calls out with individual controls.
- Manually add:
  -Wundef
  -Wswitch-enum
- Manually add and then diable in the unittests because of XCTest's headers:
  -Wreserved-id-macro
  -Wdocumentation-unknown-command
- Manually add -Wdirect-ivar-access, but disable it for the unittests and in
  the library code (via #pragmas to suppress it). This is done so proto users
  can enable the warning.
@ruslo
Copy link

ruslo commented Aug 2, 2016

I created this by looking at the changes between v3.0.0-beta-2 and v3.0.0-beta-2-hunter-2 and making the equivalent changes on top of the v3.0.0 tag

Have you tried git merge?

I don't think this is the best way of doing it

Fork branches should match upstream, so hunterized/master to upstream/master must be always merged by git merge --ff-only. Patches should go to hunter branch. As a result both git diff master..hunter and git log master..hunter will contain only Hunter related changes.

@Osse
Copy link
Author

Osse commented Aug 2, 2016

I agree. But the hunter branch in this repo does not contain the hunter-related changes at all. git log hunterized/master..hunterized/hunter is empty, and in fact hunterized/hunter is behind hunterized/master, which in turn is behind upstream/master. The changes that make up the "hunterization" of v3.0.0-beta-2 are found with git log hunterized/master..hunterized/v3.0.0-beta-2-hunter.

I suspect the branch hunterized/v3.0.0-beta-2-hunter is what hunterized/hunter should have been. The tip of hunterized/v3.0.0-beta-2-hunter is tagged v3.0.0-beta-2-hunter-2.

osse/master is equal to upstream/master. and there is only one commit in the range osse/master..osse/hunter, which is where I do the necessary changes to make it Hunter-compatible. I could have called this branch osse/v3.0.0-hunter which fits how it was done for beta 2, but does not fit how you say it should be.

@ruslo
Copy link

ruslo commented Aug 2, 2016

But the hunter branch in this repo does not contain the hunter-related changes at all

This is how things should look like :) @tatraian can you consult us with the current state?

I could have called this branch osse/v3.0.0-hunter which fits how it was done for beta 2, but does not fit how you say it should be

It should fit upstream. Is v3.0.0 a separate branch or just a tag from branch master?

@Osse
Copy link
Author

Osse commented Aug 2, 2016

The upstream tag v3.0.0 is on the upstream master branch.

In this fork there is a branch named v3.0.0-beta-2-hunter containing Hunter-related changes. The tip of this branch has the tag v3.0.0-beta-2-hunter-2. Earlier on that branch is also the tag v3.0.0-beta-2-hunter-1.

@ruslo
Copy link

ruslo commented Aug 2, 2016

@Osse okay, can you please do master -> master pull and create branch hunter starting from upstream release tag. You have permissions now.

@Osse
Copy link
Author

Osse commented Aug 8, 2016

Thanks for having faith in me :)

I have made it so that the master branches are equal. And also pushed my hunterization of 3.0.0 and tested it. But before I push , I have a question: @tatraian added the Protobuf:: namespace. Since beta2 Google have added a namespace themselves, but it is protobuf:: with lowercase P. Using the lowercase follows upstream, but requires everyone to modify their code. What do you think?

@Osse Osse merged commit 6c0829f into hunter-packages:hunter Aug 9, 2016
@Osse
Copy link
Author

Osse commented Aug 9, 2016

It says it's closed but it isn't. I force-pushed it away (sorry). I am also waiting for an answer to this question before making things final: protocolbuffers#1931

@ruslo
Copy link

ruslo commented Aug 9, 2016

Since beta2 Google have added a namespace themselves, but it is protobuf:: with lowercase P. Using the lowercase follows upstream, but requires everyone to modify their code. What do you think?

Follow upstream. This is one more reason to push non-Hunterized patches upstream instead of keeping them in fork. Just don't forget to edit wiki page.

It says it's closed but it isn't

What is closed, this pull request? I think since you pushed that code to branch manually GitHub is smart enough to figure out that changes from this pull request already merged.

@Osse
Copy link
Author

Osse commented Aug 9, 2016

Ok, I will stick to upstream as close as possible.

This is what the hunterization looks like so far: hunter...Osse:hunter

Summary: I use the most recent Hunter and Huntergate , cherry-pick a bunch of fixes to the CMake files since 3.0.0, and change ZLIB::ZLIB into ZLIB::zlib. I am still not happy as I am waiting for a response on the issue I linked to above, but with that variable set everything works just like it should.

I must admit my original pull request did a lot of stuff wrong. Also I think the CMake setup they have upstream is a bit strange, but I'm a novice.

@tatraian
Copy link
Member

Sorry Guys, I was out. I planed to update this repo as follows:
You get the original repo. Push all changes to hunterized repo.
Create new hunter branch for the wished release.
Cherry pick the modification from the previous hunter branch. There is only 4-5 commits that modifies only CMake related files.
I guess that is more simple.

@ruslo
Copy link

ruslo commented Aug 10, 2016

@tatraian sounds pretty similar to what we are trying to do as far as I understand. Probably except cherry-picking, since tags created in upstream branch master we can pull it until tag commit and do merge into hunter. Will save us few operations.

@Osse
Copy link
Author

Osse commented Aug 10, 2016

@tatraian Hi, welcome back :)

Yes, what you describe is exactly what my original plan was. However there were conflicts. Many of these conflicts were due to upstream making changes similar to what the hunterization consisted of in the first place. Which is a good thing, I suppose. So I ended up looking at the diff between v3.0.0-beta-2 and v3.0.0-beta-2-hunter-2 and applying changes onto v3.0.0 by hand making adaptations as needed. In the end the diff between v3.0.0 and v3.0.0-hunter-1 is much smaller than the diff between v3.0.0-beta-2 and v3.0.0-beta-2-hunter-2.

@tatraian
Copy link
Member

@Osse As I remember not only CMake files have been modified, I backported a small fix from protobuf to compile it with clang. But if they modified the CMakelists.txt and install.cmake then it can make rewrite easier than cherry picking...

@ruslo At the first release it is easier to make a merge to a "generic" hunter branch. But after the original project update, you have to merge into the same branch. In case of CMake files this will be hard after some releases (but you have experience in this).

@ruslo
Copy link

ruslo commented Aug 10, 2016

In case of CMake files this will be hard after some releases

Why? If merge has conflicts then cherry-pick will have them too, but if there is no conflicts instead of doing several cherry-picks you can git merge only once. Hence merge looks preferable. Anyway you can do as you wish, result should be the same: 1. git merge --ff-only for master 2. minimal hunterization diff between hunter and master.

@tatraian
Copy link
Member

You are right. That's seems easier, I prefer cherry pick because I see harder in "merge style" history what happened exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.