-
Notifications
You must be signed in to change notification settings - Fork 67
Create LibGit2Sharp.NativeBinaries package #1
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
Related to libgit2/libgit2sharp#984 |
@bording The repo should now be multi os enabled in Travis. Could you please update your PR to trigger a new build? |
@bording I've also set Appveyor up and manually triggered a rebuild of the PR. See https://ci.appveyor.com/project/libgit2/libgit2sharp-nativebinaries/history |
Looks like both of the Travis builds worked! We might want to tweak some of the libraries available. Looks like the OS X install has OpenSSL 0.9.8za and ZLIB 1.2.5 and linux has OpenSSL 1.01 and doesn't have ZLIB. The binaries probably work either way, but it might be a good idea to make sure they are same as we can make them. Alos, it doesn't look like the AppVeyor build fired with when I pushed my changed. I tweaked the script there to make sure the -pre tag is used when it is built from a PR push. |
Looking at bit more at the Travis artifacts options, the one the support out of the box is uploading to S3: Link But, we could also use their deployment stuff to tap into a different service: Link The one that looks most interesting to me would be using GitHub Releases. The only wrinkle there is that you'd have to create a tag if you want to get the binaries. That doesn't seem like too terrible of a requirement though! |
@nulltoken I'm pretty much waiting for feedback on this and the questions in the main PR. Otherwise, I believe I've gotten this as close to being done as I can! |
<package xmlns="http://schemas.microsoft.com/packaging/2010/07/nuspec.xsd"> | ||
<metadata> | ||
<id>LibGit2Sharp.NativeBinaries</id> | ||
<version>0.0.0</version> |
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.
Sorry for the dumb question. Will this be overridden by the -Version
switch of the Nuget.exe command?
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.
Yes, buildpackage.ps1
always uses the -Version
switch, so it gets overridden.
rm -rf $PACKAGEPATH$OSPATH | ||
mkdir -p $PACKAGEPATH$OSPATH$ARCHPATH | ||
|
||
cp libgit2/build/libgit2-???????.$LIBEXT $PACKAGEPATH$OSPATH$ARCHPATH |
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.
Uhm, what's with the ??????
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.
Shouldn't this be the SHORTSHA?
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.
Good point, for some reason I forgot I could use SHORTSHA there too! Will fix.
Sorry for the delay in answering. Regarding the Travis upload, as we only need some temporary storage, how about uploading to DropBox? Or taking another approach, we could first release a Windows only version of the package, solve the Travis issue, then include *nix binaries. |
after_build: | ||
- ps: | | ||
$pre = $true | ||
if (($env:APPVEYOR_PULL_REQUEST_NUMBER -eq $null) -and ($env:APPVEYOR_REPO_BRANCH -eq "master")) |
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.
As we mostly build unstable version of libgit2, maybe should we always use the "-pre" syntax for naming?
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.
The problem with giving a nuget package a prerelease version is that any packages that depend on it also become prerelease.
If we are trying to version this package to match up with the actual libgit2 versions, then yes we'd probably want these to be -pre most of the time.
However, that would force the LibGit2Sharp package to be prerelease whenever it was using one of the native binary prerelease packages.
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.
Hmrpf.. I didn't know about this pre-release viral chaining.
How sneaky would it be to use the build timestamp as the patch version (eg. 0.22.20150320121314)?
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.
Hmm, we could do that, though that does seem to be disregarding any notion of SemVer-style versioning for the package.
Obviously, if we just stuck to the officially versioned releases of libgit2, we could just use their version numbers. However, I'm assuming we want/need to be able to ship any random commit from the libgit2 repo as part of an official, non-prerelease version of the libgit2sharp package?
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.
Hmm, we could do that, though that does seem to be disregarding any notion of SemVer-style versioning for the package.
We would still be SemVer compliant. We would however abuse the patch number usage 😉
I'm assuming we want/need to be able to ship any random commit from the libgit2 repo as part of an official, non-prerelease version of the libgit2sharp package?
Indeed.
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.
We would still be SemVer compliant. We would however abuse the patch number usage
yeah, that's sort of what I meant. 😄 Using that would definitely not be the intended usage for the patch number!
Regardless, it doesn't look like an option. I gave it a try, and using a value as long as the timestamp appears not to be a valid version number.
If we just want an ever increasing number, maybe we could just use the AppVeyor build number?
One thing to keep in mind with this, if we are making non-prerelease packages and putting them up on nuget.org, then there's a better chance of people seeing that as a newer package and upgrading to it. Of course that would instantly break them since it would have the wrong native binary names, and they could just revert back.
The only other thing I could think would be to make prerelease packages up until we want to push out a new official libgit2sharp package, then you could publish a non-prerelease nativebinaries package as part of that. You could just take the latest package, open it up with Package Explorer, change the version number, and then publish it.
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.
How about actually using the AppVeyor build number as the major version (eg. 245.0.0)? Shouldn't this prevent the native package from being upgraded by the user?
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.
A larger version number, even if it's a major version will still show up as an upgrade in the NuGet dialog, or if updated from the commandline.
If we want non-prerelease version numbers for the binaries package, then I think I'm going to have to change the LibGit2Sharp nuspec file. Right now I have it automatically including the dependency.
That's useful because the file doesn't need to be updated when the nativebinaries package gets updated, but nuget puts a versioning rule in there that allows for upgraded packages.
I'm going to look and see if there is some way to alter what it includes by default, but if there isn't a way, we could change to explicitly listing the dependency in the nuspec file, and giving it a more restrictive version rule:
http://docs.nuget.org/create/versioning#Specifying-Version-Ranges-in-.nuspec-Files
That won't prevent the "newer" nativebinaries packages from showing up as an upgrade, but it will cause and error when trying to upgrade and stop it from happening.
Seems like the best way forward, but that is likely going to mean there is one more manual step to remember when upgrading the nativebinaries package in the main repo. 😦
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.
Ok, so it looks like this will be a bit easy to manage then I was originally thinking. Take a look at my latest commit on the main PR. It added an allowedVersions property to the packages.config file, which constrains the version to the one listed in the file.
When the package is built, that same version constraint is used for the native binaries dependency!
That means we can version the native binaries package however we want, and upgrading to a newer version would be prevented, and it only means making a change to a single file (packages.config) to make it all work!
Ah, bypassing Travis' artifacts support and using something like that should work. If you want, I can look into adding that it.
That's up to you on the timing. The packages that come out of the AppVeyor builds would be ready to go for a Windows-only release. If you'd rather get this going with just the windows binaries first, we could do that. |
Looking at the S3 pricing, it doesn't look like it would be all that expensive to just set up an account and use Travis' built-in artifacts upload support vs. spending more time to get something else working. |
Btw, it looks like AppVeyor still might not be looking at this repo properly. It's not showing up in the GitHub CI stuff, and the direct site link to the project shows that it's not auto-building when I push to the PR branch. |
Tweaked the webhook a little bit. Can you give it another try and see if that works better? |
I've created a libgit2 org at at bintray.com (https://bintray.com/libgit2) and a container to host those temp packages from Travis (https://bintray.com/libgit2/compiled-binaries). Also found some potential helper scripts and guidance regarding automated upload to bintray (globaleaks/globaleaks-whistleblowing-software#310) @bording Could you please sign up and ping me so that I add you as a member of this org? |
Done! I should be able to take a look at integrating this (and get the main PR mergeable again) this weekend. |
Temp upload bintray repo and hardcoded creds for now to test.
Despite separate packages in a bintray repo, filenames must be unique across the entire repository!
Ok, I have the bintray integration working as far as I can get it, because I don't have access to the libgit2 Travis settings to add environment variables. While testing, I was uploading binaries to the linux and osx packages here if you want to see what it looks like. Right now, I have the bintray API creds hardcoded in the script (and I've revoked that API key now that I'm finished testing.) Assuming we want to keep those private, then you'll need to add Travis environment variables with the appropriate value from the settings page. One downside to using environment variables that way is that the Travis docs indicate that the variables aren't accessible by builds launched from PRs. Probably not a huge deal, but that does mean we won't get access to Travis artifacts from any PRs. I should probbly add a TRAVIS_PULL_REQUEST check to the uploadbinaries.sh script to skip it when it's a PR. |
@nulltoken Had a chance to look at this yet? |
I've been a bit under lately. I'll take a look at it during the week-end. Sorry for the delay 😖 |
before_install: | ||
- date -u | ||
- uname -a | ||
- env | sort |
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.
Considering the secure variable we're going to have to support, maybe should we drop this.
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.
I've found that env list to be pretty useful while working on this, so I'd hate to lose it.
I can modify that to exclude the secure variable line and keep the rest.
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.
👍
Superseded by #3 |
Here is everything I have so far from my original bording/libgit2sharp.nativebinaries repo