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

(#2400) Replace UppercuT with Chocolatey.Cake.Recipe #2706

Merged
merged 21 commits into from
Jun 23, 2022

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Apr 29, 2022

Description Of Changes

This PR removes the existing UppercuT build, and replaces it with Chocolatey.Cake.Recipe, which is an opinionated
set of build scripts that use Cake scripting under the hood. This recipe is already being used on a number of projects
internally, and it makes sense to use it on our open source projects as well.

Motivation and Context

The UppercuT build is starting to show its age, and isn't easy to update and maintain.

The Chocolatey.Cake.Recipe package means that the same build process can be shared across a number of projects
with each project only needing to provide the project specific pieces of information to make the build work.

Testing

I have built this locally on my machine (which has access to the signing certificates) and the generated assembly contains what is expected, i.e. correctly signed choco.exe, chocolatey.dll, PowerShell scripts, etc.

In order to test this properly, we will need to get this onto a complete integration tests execution internally, and verify that all the tests work as expected.

NOTE: The production CI build won't work until this PR is merged, as there were changes to the CI configuration that won't take affect until the merge happens.

Change Types Made

  • Bug fix (non-breaking change)
  • Feature / Enhancement (non-breaking change)
  • Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Fixes #2400

Change Checklist

  • Requires a change to the documentation
  • Documentation has been updated
  • Tests to cover my changes, have been added
  • All new and existing tests passed.

@gep13 gep13 linked an issue Apr 29, 2022 that may be closed by this pull request
2 tasks
@gep13 gep13 marked this pull request as draft April 29, 2022 13:27
@corbob
Copy link
Member

corbob commented Apr 29, 2022

@gep13 as discussed on Slack, I have pushed to your branch here two commits. The first commit updates the pester test infrastructure to look for the new nupkg location, and updates the Vagrantfile to prevent most of the output from the build due to what appears to be a buffer overflow issue with vagrant. The second commit is intended for you to rebase away when you go in and update the recipe.cake to specify the file more directly.

Tests are running successfully as they did with UpperCuT, and can be run by navigating to ./tests and running vagrant up (note: only tested so far with virtualbox).

@TheCakeIsNaOH
Copy link
Member

TheCakeIsNaOH commented Apr 29, 2022

For the docker-build error, I am getting the same message about LibGit2Sharp locally. It should theoretically be fixed by updating the version of Cake.Git used. Bumping the version to 1.1.0 fixed the error for me.

I'm running into a gitversion error when the LibGit2Sharp error is fixed, but later on.

@gep13
Copy link
Member Author

gep13 commented Apr 30, 2022

@TheCakeIsNaOH can you shed any light on why this action is being marked as successful, even though the build itself fails:

https://github.com/chocolatey/choco/runs/6241475808?check_suite_focus=true#step:3:721

I think I am missing something in terms of how Cake is being called, but I am not sure.

Ignore the gitversion error, I am working on that, but leaving this in as a test to actually have the build marked as failed.

@TheCakeIsNaOH
Copy link
Member

@gep13 perhaps the build.sh script is exiting with a zero exit code, even though cake is failing?

Try setting the unofficial strict mode for bash, via adding set -euo pipefail right after #!/usr/bin/env bash
http://redsymbol.net/articles/unofficial-bash-strict-mode/

@gep13
Copy link
Member Author

gep13 commented Apr 30, 2022

Trying now...

Thanks!

@gep13
Copy link
Member Author

gep13 commented Apr 30, 2022

@TheCakeIsNaOH that did it, thank you! All the builds are now correctly failing! Now we need to get them all passing 😄

@TheCakeIsNaOH
Copy link
Member

Yup, at least there is progress being made.

Don't forget to add that line to the other .sh scripts as well.

@gep13 gep13 force-pushed the replace_uppercut branch 3 times, most recently from eaeb8e0 to f1bf8bd Compare May 3, 2022 08:56
@gep13
Copy link
Member Author

gep13 commented May 3, 2022

@TheCakeIsNaOH @vexx32 @corbob @JPRuskin @Windos I "think" I have most of the pieces in place here (aside from the failing Docker build)...

Would be good if you could all take a look at the changes that I have made here, take it for a spin, and let me know if you have any questions or concerns.

Thanks!

@TheCakeIsNaOH
Copy link
Member

What about the test.bat/test.sh? The test.yml workflow would currently fail since those are missing.

@gep13
Copy link
Member Author

gep13 commented May 3, 2022

That is a good catch. We will likely need to add another task in the Recipe to cover that.

@gep13 gep13 force-pushed the replace_uppercut branch 3 times, most recently from 9b0f662 to 87e201a Compare May 6, 2022 10:25
@gep13
Copy link
Member Author

gep13 commented May 6, 2022

@TheCakeIsNaOH @AdmiringWorm so, I have all the builds now succeeding, although I have now broken the local build of the docker image 😢

The issue here is that by introducing the environment variables from the GitHub Actions provider to the docker file (in order to get it to understand that it was building on GitHub actions), due to the way that GitVersion works, it now also thinks it is running on GitHub Actions when it is running locally.

Ideally, what I want to do is to conditionally do this work:

ENV GITHUB_ACTIONS=$github_actions
ENV GITHUB_REF=$github_ref
ENV GITHUB_REPOSITORY=$github_repository
ENV GITHUB_BASE_REF=$github_base_ref
ENV GITHUB_HEAD_REF=$github_head_ref
ENV GITHUB_RUN_NUMBER=$github_run_number

Based on the build arguments being passed in. If they are not there, I don't want the environment variables to be set at all.

Any ideas?

@TheCakeIsNaOH
Copy link
Member

@gep13 this is not something I have had to do before, but here are some possibilities of what might work:

@gep13
Copy link
Member Author

gep13 commented May 6, 2022

I had the same thought about using a separate Dockerfile, but I am not sure if I am a fan of that approach 😄

I will have a play with one of the conditional approaches in that Stack Overflow question (I actually think I read that question earlier today) to see if anything helps there.

We are so close with all of this I think!

@TheCakeIsNaOH
Copy link
Member

TheCakeIsNaOH commented May 7, 2022

This is now working for me locally on Windows, Debian, and in Docker! 🥳

@gep13
Copy link
Member Author

gep13 commented May 8, 2022

Woot! Thanks for confirming!

@gep13
Copy link
Member Author

gep13 commented May 8, 2022

@TheCakeIsNaOH @vexx32 @corbob @JPRuskin @Windos I need to rebase some things, but I believe that this is now ready for reviewing/merging.

Once I have finished rebasing, I will mark this PR as ready.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Show resolved Hide resolved
@gep13 gep13 force-pushed the replace_uppercut branch 2 times, most recently from 4dfdb52 to a26e9fe Compare May 9, 2022 12:44
@gep13 gep13 marked this pull request as ready for review May 9, 2022 12:44
@gep13
Copy link
Member Author

gep13 commented May 9, 2022

@TheCakeIsNaOH @vexx32 @corbob @JPRuskin @Windos this is now ready for review. Have fun, I know I did! 🎉

cake.config Outdated Show resolved Hide resolved
build.ps1 Outdated Show resolved Hide resolved
gep13 and others added 21 commits June 23, 2022 10:58
These are no longer required, as all the functionality is now being
provided by Chocolatey.Cake.Recipe.
Previously, the UppercuT build required that all tools were either
already installed on the machine, or present in the lib folder of the
repository.  This strategy is no longer required, as the
Chocolatey.Cake.Recipe already has the ability to download the tools
when required as part of the build process.
Storing the NuGet.exe is in the repository is no longer required.  This
tool will be resolved, as required, when the build runs.
Chocolatey.Cake.Recipe is quite opinionated about the folder structure
that is used when creating Chocolatey and NuGet packages.  It expects
all nuspec files to be stored in a folder structure like the following:

- nuspec
  - chocolatey
  - nuget

With the UppercuT build, this structure existed, just in a slightly
different way.  This commit updates the folder structure to be what is
required.
This includes pinning to the currently supported version of Cake,
0.38.5, and also to include a cake.config file to allow overridding of
the checks that Cake does of out of support addins (these are
knowingly being overridden at the minute).

The Cake bootstrappers that are being used in the Chocolatey GUI build
have been copied over into this repository.

Within the recipe.cake file, the project specific changes have been
made, including which files should be signed, ILMerge'd, etc.
Rather than calling into UppercuT the build entry files, i.e.
build.bat, etc, have been updated to call into Cake.
Cake outputs to a new file (and a lot to the terminal). This updates the
script used for running the tests to default to the new location.
We also update the Vagrantfile to silently absorb the output from the
build script as it seems to output too fast for vagrant to handle
consistently.
The folder structure has changed, as a result, changes to the build, as
well as what parameters are passed into the build, need to be made to
match the new way of doing things.
This will mean that the generated release notes are using the standard
look and feel that we want for all release notes.
The strucutre of the code_drop folder has changed, and the docker
portion of the build needs to be updated to reflect this new
structure.
This NuGet package is no longer required, as the work that it was doing
is now being handled within the Chocolatey.Cake.Recipe. i.e. the output
from the different project types is now copied to the correct locations
as part of the build process.
This is not currently being used, and can be removed as it will reduce
maintenance overhead going forward.
A lot of the work that was previously being done in the TeamCity settings
file is now done by the Recipe, and therefore can be removed.
In order to get the build to work correctly, we need to install a newer
version of git on the build machine (this means using the backports
registry) and also to install the .NET CLI.
In order for the Cake Build to correctly identify that it is running
on GitHub Actions, in order to assert teh correct branch name to use the
environment variables from the GitHub Actions build image have to be
passed through to the execution of Docker. We don't need all the
variables, just the ones used by Cake to make some assertions.
To account for the new way that things are executed under
Chocolatey.Cake.Recipe.
The ordering of the tasks was thrown out a bit due to the introduction
of the Prepare-NuGet-Packages and Prepare-Chocolatey-Packages tasks.
This corrects that issue, and also maps all the required files
This will mean that Cake and the other tools/dependencies won't need to
be resolved/downloaded each time.
@gep13
Copy link
Member Author

gep13 commented Jun 23, 2022

I believe that the failing Mac build here is due to this issue:

actions/runner-images#5768 (comment)

As such, I am going to say that we can ignore this failure, and move forward with the merging of this PR.

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

Successfully merging this pull request may close these issues.

BuildProcess - Replace UppercuT build scripts
3 participants