-
Notifications
You must be signed in to change notification settings - Fork 67
Create LibGit2Sharp.NativeBinaries package #3
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
Conversation
@nulltoken So as best as I can tell, I've done everything I need to, but I'm stil not getting access to the secure variables. Neither of the travis builds that fired from the branch push and the new PR are seeing them. The build logs are showing Hmm. |
Secure vars are disabled for external pull requests. In order to gain access to them, you'll have to push to this repo and create the PR between your branch here and master. |
Right, that's what I did with this new PR. It has been created from the bording/addStuff branch on the main repo, not my fork. |
- linux | ||
|
||
env: | ||
- secure: "oHrjeGkWTUiPLsdYGb4n4lI37EURXVDT1bJ4a4CXfwnFlxXjWF3ixaYqg88o5weT/L/OPhjRdf3GVqmdxPeUYNeF8ImdewcFwJ4C3jLtl2G9CGAP0xVsbyQHxjDi07MHcSbnNorQI0QSEta9bdP6EprNcoEwd/Vi1u14f5g40tk=" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bording Oh, I may have something. Following travis-ci/travis-ci#2820 (comment) led me to this BanzaiMan/travis_production_test@d24c694 where one can see no dash ("-") before the secure
key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed a fixup to test this idea out. Let's wait and see...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm. I eventually had to remove the secure variable from the travis build script and defined the key in the repository settings.
Now we've got a nice $ export BINTRAY_API_KEY=[secure]
in the build log 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And... we've got something at https://bintray.com/libgit2/compiled-binaries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. I'm still not sure what needed to change to get the secure var working as intended, because as far as I could tell from the docs I had it set up correctly.
Setting it directly in the repository settings works just as well, so I guess we won't worry about it!
Now that we've got Travis working properly with bintray, the last remaining this is to figure out how to handle the package versioning, and update appveyor.yml appropriately. |
@nulltoken Looking at the packaging versioning situation, I see three possible options: Option 1: Track the libgit2 versionIn many ways this is the option that makes the most sense, since this package is just a bundling of the native library. However, this is too restrictive since we'd then be locked into just releasing this to line up with the officially versioned libgit2 releases. Option 2: Track the LibGit2Sharp versionThis is more or less what the PR is doing now. We would build prerelease versions of the package to go with the prerelease versions of LibGit2Sharp. The benefit of this is that the version numbers of the official packages line up nicely, e.g. LibGit2Sharp 0.22.0 depends on LibGit2Sharp.NativeBinaries 0.22.0 However, the downside of this approach is more manual work to keep the versioning synchronized. Whenever LibGit2Sharp's version is bumped, the appveyor.yml of this repo has to also be changed to match. That's not too bad, but the bigger issue is that when it's time to release a new official version of LibGit2Sharp's package, the current prerelease NativeBinaries package will have to be rebuilt/reuploaded in advance just to have the correct official version number. That then ripples back to the LibGit2Sharp repo, where you have to update packages.config for the new NativeBinaries package number. Option 3: Make the version number independentThis option completely decouples the version number from everything else. I would alter the appveyor.yml to something like
and then we never worry about the version number again. Whenever a new version of libgit2 is needed, the repo just gets updated and we get a new package with whatever the version happens to be and LibGit2Sharp gets updated accordingly. We can keep the prerelease tag for any non-master builds if we want, which could be useful. Since option 1 isn't viable and because the NativeBinaries package isn't intended to be used directly by people (making it less important for the version number to convey meaning), I'm leaning towards option 3. Thoughts? |
Sorry. I realize that my question was badly worded. Let me try to elaborate further on my thoughts and see and I can share in a better way my current state of mind.
Given this, we could even refine further Option 1
Option 2 comes with too many downsides in my opinion Option 3
Currently, I'd prefer to go with |
The scenario I was thinking of is when you're testing to see if updating to a new libgit2 version fixes a problem before actually making the change. I believe I've seen some branches in the main repo like this before. The easiest way to do this would be to create a test version of the nuget package to try it out, and if you want linux and osx builds also, the simplest way to get them would be to let the CI systems build a package. Of course, you wouldn't need to push this test package up to nuget.org, you could just test it locally, but have a -pre tag on the package automatically might be helpful here. You could also skip the CI step altogether for this and run So, not super important, but I've already got the support for it in the scripts. |
👍 This lines up with everything I was thinking.
An additional problem I see with this is that it doesn't fully track the libgit2 version number as found in the version header, which lists a revision and patch version that we'd be ignoring. If we're building more often than that file changes (and they tag a new release), then package version numbers aren't useful for determining the libgit2 version.
In addition to having to predict the future version number for this work, we also run into that NuGet viral prerelease problem. The LibGit2Sharp packages would have to be prerelease also if we depend on a prerelease binary package. |
Dammit! I (once again) forgot about this :/ Long live |
@nulltoken Ok, with the package version decided on, I think this is ready to go! I'm assuming you'd prefer this get squashed down to a single commit? Once we get this finished, the next step is to get an actual package up on nuget.org. Then I can update my PR on the main repo to use it instead of my test package, and it will be ready to merge also! |
Yes, please 🙏 Unless anyone has any further comment, it looks like an amazing piece of engineering ready to be merged in. |
|
Squashed and ready for merging! |
Create LibGit2Sharp.NativeBinaries package
"Merge into master" build logs were
Downloaded
Opened the downloaded AppVeyor package with NuGet Package explorer
Pushed the resulting package to NuGet gallery |
Recreating PR #1 from a branch on the upstream repo instead of my fork.