Use GitHub Releases for official builds #946
Conversation
This get ride of AppVeyor for official builds once and for all. As people have been using AppVeyor for custom build and as number might clash with official releases, the minor version of the project was incremented. AppVeyor will still be used for PRs. Also to avoid more confusion between AppVeyor builds and official one, AppVeyor builds are now postfixed with "-prXXX".
marysaka
added
enhancement
|
What is the maintenance plan for AppVeyor moving forward? Seems like this will require twice the work to maintain if we ever add new build requirements or features, as the changes will need to be done against both GitHub Actions and AppVeyor. Is GitHub Actions not capable of building the PRs also? What was the reasoning behind moving away from AppVeyor? It seems, at least from what I've seen and heard, that GitHub Actions is a less capable and featured solution, what are some of the merits of moving to GitHub Actions? |
|
GitHub Actions is already building PRs, the only missing feature is an artifacts storage. (the feature is here but limited to 250MB per repository) The major reason behind moving away from AppVeyor for master building is to have proper releases, not build artifacts. AppVeyor isn't designed to be a long term storage for releases as every builds expires after 6 months. Add to this that AppVeyor sometime silently generating wrong artifacts and that runner takes quite some time to start (already had to wait 15 min for a build to start) and you have my reasons. PS: If you want to unify the workflow, we could probably use MSBuild task as you said in private or we can also do docker images like RPCS3. We could also consider moving to Azure Pipeline. |
| { | ||
| public class ArtifactInformation | ||
| { | ||
| public OperatingSystem Os { get; private set; } |
There was a problem hiding this comment.
Don't forget to align names if you want to align {}
| { | ||
| switch (part) | ||
| { | ||
| case "win": |
There was a problem hiding this comment.
Maybe you can put return on the same line and align them?
| { | ||
| switch (part) | ||
| { | ||
| case "x64": |
There was a problem hiding this comment.
Maybe you can put return on the same line and align them?
|
|
||
| public static ArtifactInformation FromFileName(string fileName) | ||
| { | ||
| string fileNameWithoutExtension = Path.GetFileNameWithoutExtension(fileName); |
There was a problem hiding this comment.
Sometime you align assignment, sometime you don't align name, and sometime you align everything.
You should make a choice :P
| artifactInformation.BuildType = BuildType.ProfileRelease; | ||
| } | ||
|
|
||
| string targetConfiguration = isProfile ? fileNameParts[3] : fileNameParts[2]; |
| } | ||
|
|
||
| string commitSha = Environment.GetEnvironmentVariable("GITHUB_SHA"); | ||
|
|
| } | ||
|
|
||
| string[] releaseRepositoryParts = releaseRepository.Split("/"); | ||
|
|
| GitHubClient client = CreateGitHubClientInstance(releaseToken); | ||
|
|
||
| // Create a new release | ||
| NewRelease newRelease = new NewRelease($"build-{commitSha}"); |
There was a problem hiding this comment.
NewRelease newRelease = new NewRelease($"build-{commitSha}")
{
Name = blabla,
Body = blabla,
etc...
};| newRelease.Draft = true; | ||
| newRelease.Prerelease = false; | ||
|
|
||
| Release release = await client.Repository.Release.Create(releaseRepositoryOwner, releaseRepositoryName, newRelease); |
| public VersionCore Version { get; set; } | ||
| public List<ArtifactInformation> Artifacts { get; set; } | ||
|
|
||
| private ReleaseInformation() |
There was a problem hiding this comment.
| private ReleaseInformation() | |
| private ReleaseInformation() { } |
|
That's valid. AppVeyor can deploy artifacts to GitHub Releases as a post-build process, so if you wanted to have a more solid release process, that would have been a solution also. I wasn't aware that we were having any issues with AppVeyor builds generating incorrect artifacts, that sounds more like a misconfiguration than an issue with AppVeyor to me. I'm open to moving build and CI providers, I just want to make sure we're doing it for the right reasons, we understand what those reasons and issues are, and we make an informed decision on what the best platform is going forward. To me, personally, GitHub Actions is not ready for prime time yet, and is missing too many features and has too many limitations to be a primary choice for CI, but without understanding what our requirements are, we can't really make a decision on the best solution. For now, this looks fine, however I don't really like the idea of splitting PRs and releases across two different build platforms. I also don't think the Ryujinx.Actions project belongs in the main Ryujinx repository. I think it would be best to create another repository and store that code there, it doesn't really have anything to do with Ryujinx, and just adds clutter to the already cluttered repo. |
|
Using GitHub releases for storing every build of master seems like it would get cluttered rather quickly. In general I'd assume that releases were manually selected instead of done for every commit to master. What do you mean by "incorrect artifacts", @Thog? Ideally you'd be able to run the build script locally while only using CI-Provider-specific stuff for things like posting artifacts or specifying the build configurations that should be built. |
rpcs3 works around this by having a separate repository for releases: https://github.com/RPCS3/rpcs3-binaries-win You can link to the latest artifacts like AppVeyor lets you do by making a link to the latest release: https://docs.github.com/en/github/administering-a-repository/linking-to-releases |
|
Closing for now, I plan to redo it later. As @Margen67 said, storing GitHub Releases for every commit is entirely fine and is already being used by RPCS3 so I will keep most of the current design but add into consideration multiple channel releases. |
This get ride of AppVeyor for official builds once and for all.
As people have been using AppVeyor for custom build and as number might clash with official releases, the minor version of the project was incremented.
AppVeyor will still be used for PRs.
Also to avoid more confusion between AppVeyor builds and official one, AppVeyor builds are now postfixed with "-prXXX".