add all AppVeyor builds to its NuGet feed

[ctaggart]

As I mentioned at #915 (comment), here is a PR that makes all builds available via the AppVeyor NuGet feed. See
http://blog.ctaggart.com/2015/07/open-source-net-with-appveyor-nuget.html

@CharliePoole just needs to go to
https://ci.appveyor.com/project/CharliePoole/nunit/settings/nuget

and change it to something like:
https://ci.appveyor.com/nuget/nunit

The script support creating a build from a tag. If you pushed a 3.0.0-beta-6 tag, it will correctly create the packages in the AppVeyor feed. Let me know what questions you have.

[ctaggart]

It works. The artifacts are here:
https://ci.appveyor.com/project/CharliePoole/nunit/build/3.0.0-beta-6-1570/artifacts

The *.nupkg files will be in the feed. Just rename the feed and we can use the binaries.

[CharliePoole]

This looks good! It's getting late so I'll review it in detail tomorrow. One immediate question... with your test run, it looks as if we released a new beta, but of course we didn't and I can't figure out how to delete artifacts from the Appveyor web interface.

[ctaggart]

A 3.0.0-beta-6-1570 build is one that would come before 3.0.0-beta-6. You can adjust the prerelease modifier anyway you wish, but that made sense to me and it will work with NuGet updates.

[CharliePoole]

Don't get me wrong. This is a very promising way to go, but we need to review it before it goes into production, decide what frequency we want to release at, what we'll call the releases, etc.

What's a bit dismaying is that even though it's not yet merged into our master, there's now a binary release out there under our account for folks to find. That's not good. It actually looks like anybody could create a fork, request a merge and the artifacts of that merge would show up for download on our Appveyor feed, looking like an NUnit-supported release. Am I missing something or is this a security hole in Appveyor?

[CharliePoole]

Thinking about it, I guess the main problem is that you used a naming convention which, for us, signals that this is an official release. We are not following a continuous release model right now, while working toward the big bang 3.0 release in a few weeks.

If we can't get rid of the spurious beta 6 (since there never will be a beta 6) we can live with it but we shouldn't create any more published "releases" until we decide how we want to do it.

(By published, I mean anybody can follow the url and get them.)

[enricosada]

@CharliePoole the appveyour nuget packages are really good wayt to try latest master or a specific pr without the need to build from source. Is like a nightly release, only each commit.

I know the ci feed is not stable, less tested, is a work in progress and can be broken each commit, but for some workflow is good.

For example, i am trying now to upgrade https://github.com/Microsoft/visualfsharp from nunit 2.6 -> 3 (i use latest best) but i'd like also to test coreclr. With the package built from appveyor is easy, i just add the feed and change the package version.

[rprouse]

@CharliePoole, I understand your concern with the artifacts being confused with official releases, but I also agree with @enricosada that we need a way of putting the most recent builds of our code up as packaged NuGet files for testing. For example, I currently needed to do a local build and package to get the fixes I needed for the Xamarin WinPhone runner. It would have been nice to be able to reference a nightly build. This is what I was hoping to do with the MyGet feed. It wasn't intended as official builds, but to house nightlies.

I think the main problem with this PR is that we are using the beta-6 tag in the artifacts. Some other teams use the beta modifier, but we are already using it for our official builds. Other teams also use nightly, CI or Dev to indicate that they are not official releases. I think we should change 3.0.0-beta-6-{build} to be something like 3.0.0-Dev-{build}.

I would also propose that we only publish artifacts from the master branch. Like you, I don't like that anyone can create a PR causing potentially malicious code to be packaged and published.

How does this sound?

[CharliePoole]

I understand the value. We were planning on doing this post-3.0 through myget. It was new to me that Appveyor has built-in feeds. We have been following more of a controlled release model for the first release (remember that "3.0" is essentially a new 1.0 product) because the minimal functionality needed for release from the new codebase had to be at least as good as NUnit V2. Admittedly, we have passed that point a while back except for the absence of a gui.

Anyway, the problem I am seeing (or imagining, maybe) is that you and others can now see a build from a pre-merge test of forked code. I have no doubt that @ctaggart 's code is safe to use but I do worry that any non-committer can create a download that looks the same as one we might release. If I go to the URL it appears that NUnit has just released beta-6. I know that it wasn't intended it to look that way, due to the addition of a suffix, but I'm pretty sure that's how most people would take it.

But the issue isn't really this particular name, but the fact that it was possible for a non-committer to create it and call it anything he wanted.

Of course, as I said above, I didn't even know that Appveyor had this capability so there may be some safeguards we can use. I'll read up on that after my morning meeting.

[rprouse]

Does on branch work with artifacts, or just deployments? If not, we could use a conditional build configuration, http://www.appveyor.com/docs/branches#conditional-build-configuration

on:
      branch: master

[CharliePoole]

Looks like a possibility but doesn't change the fact that anyone can fork
us and change appveyor.yml

On Mon, Oct 26, 2015 at 8:44 AM, Rob Prouse notifications@github.com
wrote:

In appveyor.yml
#932 (comment):

- ps: $wc = New-Object 'System.Net.WebClient'

- ps: $dir = "bin\Release\Results"

- ps: $uri = "https://ci.appveyor.com/api/testresults/nunit/$($env:APPVEYOR_JOB_ID)"

- ps: foreach($item in (dir $dir "*.xml")) { $wc.UploadFile($uri, $item.FullName) }

+artifacts:

Does on branch work with artifacts, or just deployments? If not, we could
use a conditional build configuration,
http://www.appveyor.com/docs/branches#conditional-build-configuration

on:
branch: master

—
Reply to this email directly or view it on GitHub
https://github.com/nunit/nunit/pull/932/files#r43010247.

[CharliePoole]

Exactly right. Except...

We can't change what's out there or even delete it.

We can set up our name standard in the script, but anyone can fork us and
change the script.

Seems like this is not a question of whether we should have CI builds... we
were planning that anyway. It's more about an odd hole in the appveyor
implementation.

On Mon, Oct 26, 2015 at 8:30 AM, Rob Prouse notifications@github.com
wrote:

@CharliePoole https://github.com/CharliePoole, I understand your
concern with the artifacts being confused with official releases, but I
also agree with @enricosada https://github.com/enricosada that we need
a way of putting the most recent builds of our code up as packaged NuGet
files for testing. For example, I currently needed to do a local build and
package to get the fixes I needed for the Xamarin WinPhone runner. It would
have been nice to be able to reference a nightly build. This is what I
was hoping to do with the MyGet feed. It wasn't intended as official
builds, but to house nightlies.

I think the main problem with this PR is that we are using the beta-6 tag
in the artifacts. Some other teams use the beta modifier, but we are
already using it for our official builds. Other teams also use nightly, CI
or Dev to indicate that they are not official releases. I think we should
change 3.0.0-beta-6-{build} to be something like 3.0.0-Dev-{build}.

I would also propose that we only publish artifacts from the master
branch. Like you, I don't like that anyone can create a PR causing
potentially malicious code to be packaged and published.

How does this sound?

—
Reply to this email directly or view it on GitHub
#932 (comment).

[rprouse]

Switch to -Dev?

[CharliePoole]

Clearly, we can change the name. Not the issue for me though.

On Mon, Oct 26, 2015 at 8:46 AM, Rob Prouse notifications@github.com
wrote:

In appveyor.ps1
#932 (comment):

@@ -0,0 +1,35 @@
+# the version under development, update after a release
+$version = '3.0.0'
+$modifier = '-beta-6'

Switch to -Dev?

—
Reply to this email directly or view it on GitHub
https://github.com/nunit/nunit/pull/932/files#r43010445.

[CharliePoole]

For me, what's needed is for appveyor to somehow make it clear what is
being built when somebody is looking at the artifact display. I'll sign off
for now and come back when I'm back home looking at the code.

On Mon, Oct 26, 2015 at 8:47 AM, Charlie Poole charliepoole@gmail.com
wrote:

Clearly, we can change the name. Not the issue for me though.

On Mon, Oct 26, 2015 at 8:46 AM, Rob Prouse notifications@github.com
wrote:

In appveyor.ps1
#932 (comment):

@@ -0,0 +1,35 @@
+# the version under development, update after a release
+$version = '3.0.0'
+$modifier = '-beta-6'

Switch to -Dev?

—
Reply to this email directly or view it on GitHub
https://github.com/nunit/nunit/pull/932/files#r43010445.

[rprouse]

Interesting, I didn't even think about the possibility of a user being able to fork a repository, create a pull request changing appveyor.yml and causing changes to be published. In the AppVeyor settings, there is an option Do not publish NuGet package artifacts to account/project feeds on Pull Requests. I have enabled that for nunit. That should take care of most of your security concerns. People could still go in view our builds, find artifacts and download them directly, but that is unlikely. It is more likely that they will use our CI feed.

If we also make the change that we don't create artifacts, we can watch for changes to appveyor.yml in the future. Less than ideal, but that vulnerability already exists as this PR points out.

[ctaggart]

Good morning,

Yes, that setting will disable publishing artifacts from PRs. However, I would recommend not doing that. An AppVeyor NuGet feed is a development feed. The vast majority of users will wait for a tagged release build that is promoted to the NuGet Gallery. The select few that use the AppVeyor NuGet feed know what PRs are. They are clearly marked on the build history page. It is useful to be able to test a PR from another developer.

I would recommend taking a look at other open source projects that do this already. Here is one:

https://github.com/fsharp/FSharp.Compiler.Service
https://ci.appveyor.com/project/fsgit/fsharp-compiler-service/history

I would recommend against 3.0.0-Dev-{build}, since dev comes after beta alphabetically. alpha or just a or b would work. Most projects I setup just use b.

Later, if would want a NuGet feed that only has builds from master, you can easily have AppVeyor deploy them to MyGet.

[CharliePoole]

@rprouse Yes, I saw that setting, but wanted to have a chance to figure out the implications of its use before using it. @ctaggart has presented us with an entirely new (to us) option, so we should take the time to figure out the implications. I only raised the one question because the (apparent) beta-6 release is already out there and I didn't want to create any more of them until we reviewed this.

#ctaggart If the display on the build history page were really clear, I wouldn't have an issue with this. However, what I see is that there is a build from master marked as 3.0.0-beta-6-1570. It seems to look exactly the same as it will look when we eventually merge it into master except for the name of the developer. So I think this really is something we have to think about and figure out.

I'll look at some other projects and at the detail of your PR (finally, now that I'm back to my desk) and comment on it. I'll also look at other projects as you suggest.

That said, however, I have converted a lot of projects from traditional big-bang releases to continuous release and I can tell you that it's generally more than just changing a setting in a build script! Just give us a chance to digest this.

Regarding the naming of packages, it will be easier after 3.0, when we switch to using the odd/even pattern as planned. We'll be doing bug fixes under 3.0 and development on 3.1. We can invent any sort of naming pattern that will work for us and won't have to worry about alphas and betas any longer. However, I think dev actually works quite well at the present time, as does ci. We know that the next release will be rc, which comes after pretty much anything.

More later...

[CharliePoole]

Need to set DisplayVersion as well. It's used in the msi dialogs.

[CharliePoole]

As far as code review goes, this looks great with the only issue being DisplayVersion for the msis.

We have to make a few policy decisions before merging it, that's all. Ball's in our court. I think we can resolve this in a day or two.

[ctaggart]

Sounds good.

[rprouse]

@CharliePoole, speaking of semantic versioning, I was wondering if our plan to have odd number releases will play nicely with NuGet. Our plan is that 3.1 was going to indicate development in progress, but unless we add a suffix, NuGet won't treat it as a pre-release. If we are going to have to add a suffix, then maybe we should just stick with the beta (or dev) for 3.1 eventually removing the suffix to promote 3.1 to a full release.

I like that AppVeyor creates packages in a consistent and predictable manner, removing the potential for human error. Based on that, what about this idea,

• Once we release 3.0, we modify appveyor.yml to do branch based builds, one branch for 3.0 release fixes and another for 3.1 development work.
• Both of those branches will create artifacts (or maybe just dev?).
• The release branch will create artifacts in the form 3.0.x.{BuildNumber}. This will indicate that they are releasable. We will increment the x manually. Team members can download an test these candidate releases. If/when we are ready to do a new 3.0 release, we push those artifacts to GitHub releases and NuGet. The AppVeyor artifacts will be our backup and history and because the build is done automatically, we know that no steps were missed.
• The dev branch will produce artifacts in the form 3.1-beta-{BuildNumber}. These will be available in the AppVeyor NuGet feed as nightly/CI builds for testing and possibly consumption by other NUnit repositories for their dev work. At points when we are comfortable, we will promote select betas from AppVeyor to NuGet to serve as official betas in the same way we do now except the beta number will not increment by 1 and it will have been built by AppVeyor, not us.
• Eventually, we will consider 3.1 release ready, remove the beta tag and merge to the release branch.

[CharliePoole]

I was writing up something about our future versioning, for us to discuss after the 3.0 release. I didn't want to slow us down by putting it out now. But maybe it's time to talk about it. I'll finish that up and put it out on the developer list. (Anybody listening, get on the developer list if you aren't already.)

One easy solution to the pre-release problem is to not put the unstable builds on nuget at all, but to have them elsewhere... like on myget.org or on the Appveyor feeds. I'll take a look at what some other projects using this do. Bear in mind that with the unstable/stable concept, there would not actually be any 3.1 releases - only 3.0 and eventually 3.2. Better I write this up more fully though.

Frankly, I'd rather get away from the idea of alphas and betas and move to a more continuous - or at least more frequent - release system on both the unstable and stable branches. Assuming all the new code had been exercised by bleeding edge users who use the dev branch, then releasing to master should not be a big deal.

Come to think of it, I should write this up on the wiki where it's more permanent and then initiate a discussion of it on the mail list.

I was thinking the dev branch would be 3.1 and the master branch 3.0. Until we we ready to release. We would merge dev into master and change master to 3.2. Dev would become 3.3

[CharliePoole]

@ctaggart

"A 3.0.0-beta-6-1570 build is one that would come before 3.0.0-beta-6."

That's not how I read the documentation. My understanding is that only the first hyphen counts as a separator and the subsequent hyphens are just characters in the pre-release suffix, which is sorted in lexical order.

I believe the SemVer spec would make beta-6 and beta-6.1570 work the way you say, but I don't think nuget implements that part of the spec.

Of course, you may have evidence to the contrary, which is why I'm raising the question. :-)

[rprouse]

@CharliePoole, I would like to get back to this PR when we have some time. Based on all of the discussions we have had, I think we can change it to fit with our final decisions.

[CharliePoole]

@rprouse I'm finding it hard to take time out for NUnit while on my family trip. Next comes thanksgiving!

I mention that because I'm working on the Cake script and my working assumption was that we would want to get that in place first. However, thinking about it, I realize we could work with this PR as an interim solution so long as we can decide what to call the CI releases.

I'll set Cake aside for the moment and pull this down to look at. I should be able to get back to you later today.

Maybe having CI builds would be a way to resolve some of the high- to critical-pri issues we have run into.

[CharliePoole]

@ctaggart Thanks for this. We're using it but are working in a copy I made so we can experiment with various naming conventions. See PR #1054.