Argo: v2.9.5 brew binary somehow getting replaced by dirty

[alexec]

Summary

brew install argo should give me a working binary - but the one I actually get does not have the static files.

Diagnostics

From brew:

argo: v2.9.5+5759a0e.dirty
  BuildDate: 2020-08-07T21:09:01Z
  GitCommit: 5759a0e198d333fa8c3e0aeee433d93808c0dc72
  GitTreeState: dirty
  GitTag: v2.9.5
  GoVersion: go1.14.6
  Compiler: gc
  Platform: darwin/amd64

From releases page:

argo: v2.9.5
  BuildDate: 2020-08-06T22:13:32Z
  GitCommit: 5759a0e198d333fa8c3e0aeee433d93808c0dc72
  GitTreeState: clean
  GitTag: v2.9.5
  GoVersion: go1.13
  Compiler: gc
  Platform: darwin/amd64

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

[alexec]

[alexec]

Maybe related:

https://github.com/argoproj/homebrew-tap/pull/17
https://github.com/argoproj/argo/issues/3567

[alexec]

Do we need to run

brew bump-formula-pr argo --version $VERSION

[alexec]

https://github.com/Homebrew/homebrew-core/pull/59258

[alexec]

@crunchtime-ali could I please ask for your help?

[crunchtime-ali]

@alexec Sure. I will have a look at it tomorrow. I think I included the static files and they should be included in the bottles which are built and distributed automatically as well.

[alexec]

@crunchtime-ali I think the fix is that Formula/argo.rb should download the binary from the releases page.

[crunchtime-ali]

@crunchtime-ali I think the fix is that Formula/argo.rb should download the binary from the releases page.

homebrew-core "bottles up" (builds binaries) via CI always from source and supplies them automatically for the current macOS and the two last ones. This will be super handy for the Argo project as we will also make sure it will build for Apple Silicone (right now go itself doesn't build). Therefore, you can't supply binaries yourself.

Let's fix the formula instead. I am calling make dist/argo which executes the server/static/files.go targets but I seem to have made an error nonetheless.
@alexec Can you execute brew reinstall argo --build-from-source --verbose --keep-tmp and have a look at the logs and temporary build-folder and see what is going wrong? You know way better how the argo build works and can hopefully spot the problem immediately.

[crunchtime-ali]

I checked why the git status is dirty and was able to determine that it considers the tree to be dirty because of .brew_home folder. Can you add .brew_home to .gitignore to fix that problem or should I create a PR? @alexec

[alexec]

Can I push to understand why we can’t use the same binary we attach to the releases page? Rather than rebuild, is there anyway we can build the correct binary as part of the Makefile?

[crunchtime-ali]

Can I push to understand why we can’t use the same binary we attach to the releases page?

From https://docs.brew.sh/Acceptable-Formulae#we-dont-like-binary-formulae:

Our policy is that formulae in the core tap (homebrew/core) must be open-source with an Debian Free Software Guidelines license and either built from source or produce cross-platform binaries (e.g. Java, Mono). Binary-only formulae should go to homebrew/cask.

homebrew-cores CI process builds binaries for catalina, mojave and high_sierra automatically.

class Argo < Formula
  [...]

  bottle do
    cellar :any_skip_relocation
    sha256 "b8db9382788c4305eda6d2ee651cd6384dc4b7e5f163c23fc703cd5158abfbde" => :catalina
    sha256 "4ac01435917d55a35f7d60dc6d861f9332f8319ab71a78499e2725c2f0e03687" => :mojave
    sha256 "2ba249f5d2a348b140dc07de880f1aff21c75e100e2342cd5dcab8dca2390b12" => :high_sierra
  end

Rather than rebuild, is there anyway we can build the correct binary as part of the Makefile?

Yes, I checked why the git status is dirty and was able to determine that it considers the tree to be dirty because of .brew_home folder which homebrew creates inside the project directory for some reason. Can you add .brew_home to .gitignore to fix that problem or should I create a PR? @alexec

[alexec]

Our policy is that formulae in the core tap (homebrew/core) must be open-source with an Debian Free Software Guidelines license

We meet that requirement.

and either built from source or produce cross-platform binaries (e.g. Java, Mono). Binary-only formulae should go to homebrew/cask.

We meet that requirement. The binary we attach to the release page is built from source. I think they are saying - you must not just attach random binaries - you must be able to demonstrate how they are built.

I think it would be acceptable to use the binary.

[gromgit]

Homebrew's requirement is that formulae must be built from source _by the Homebrew CI_. This ensures that working builds can be reproduced for whatever reason, especially by users who don't want to trust any supplied binaries.

[alexec]

@gromgit can you please point me to the page that says that? Thank you.

[gromgit]

It's in the passage itself:

formulae in the core tap (homebrew/core) must be [...] either built from source or produce cross-platform binaries

i.e. the formula build itself should start with the source code, not simply package prebuilt binaries.

[alexec]

@gromgit - thank you - I can see that that - while what you say is not explicit - it is implied.

@crunchtime-ali I can see that we need to build one binaries for each type of OS-X. That makes a lot of sense to me. I don't really care where they're built, as long as they're reliable.

Can I ask you to create a PR to ignore the Brew files that make the build dirty? Our Makefile looks at the environment variable `CI' to determine whether or not to create an empty file, or actually build the static files. It maybe that Homebrew CI sets that variable too.

I suggest we add a replace the CI variable to the Makefile called 'STATIC_FILES', and this is also set this explicitly in the .github/workflows/ci-bulid.yaml

Thoughts?

[crunchtime-ali]

@gromgit - thank you - I can see that that - while what you say is not explicit - it is implied.

@crunchtime-ali I can see that we need to build one binaries for each type of OS-X. That makes a lot of sense to me. I don't really care where they're built, as long as they're reliable.

Can I ask you to create a PR to ignore the Brew files that make the build dirty? Our Makefile looks at the environment variable `CI' to determine whether or not to create an empty file, or actually build the static files. It maybe that Homebrew CI sets that variable too.

I suggest we add a replace the CI variable to the Makefile called 'STATIC_FILES', and this is also set this explicitly in the .github/workflows/ci-bulid.yaml

Thoughts?

I created https://github.com/argoproj/argo/pull/3801. Maybe we should call the variable SKIP_STATIC_FILES instead of STATIC_FILES though because right now those STATIC_FILES are built when its value is false only. We could also flip all conditions in the Makefile to make it work with STATIC_FILES. What do you think @alexec

@gromgit Thanks for chiming in

[alexec]

Still a problem with v2.10.0

argo: latest+195c6d8.dirty
  BuildDate: 2020-08-18T23:46:34Z
  GitCommit: 195c6d8310a70b07043b9df5c988d5a62dafe00d
  GitTreeState: dirty
  GitTag: v2.10.0
  GoVersion: go1.15
  Compiler: gc
  Platform: darwin/amd64

[alexec]

@crunchtime-ali FYI - this is still incorrect.

[alexec]

195c6d8310a70b07043b9df5c988d5a62dafe00d is v2.10.0 👍

[alexec]

I thin this will be fixed for v2.11.0

[crunchtime-ali]

@alexec I will check it out once 2.11.0 is released.

[alexec]

Available for testing in v2.11.0-rc1.

[crunchtime-ali]

Available for testing in v2.11.0-rc1.

I just tested this with brew reinstall argo --build-from-source --verbose

Output of argo version is:

argo: latest+f446f73.dirty
  BuildDate: 2020-09-03T08:10:07Z
  GitCommit: f446f735b4c8c16c95f1306ad3453af7f8ed0108
  GitTreeState: dirty
  GitTag: v2.11.0-rc1
  GoVersion: go1.15
  Compiler: gc
  Platform: darwin/amd64

I will try to look into it until Saturday.

[alexec]

==> Upgrading 1 outdated package:
argo 2.10.0 -> 2.10.1_1
==> Upgrading argo 2.10.0 -> 2.10.1_1 
==> Downloading https://homebrew.bintray.com/bottles/argo-2.10.1_1.catalina.bottle.tar.gz
==> Downloading from https://d29vzk4ow07wi7.cloudfront.net/a2992d27e2888f34d7c035112508106f437e04bbbe7178d34f8a9908adbd4106?response-content-disposition=a
######################################################################## 100.0%
==> Pouring argo-2.10.1_1.catalina.bottle.tar.gz
==> Caveats
Bash completion has been installed to:
  /usr/local/etc/bash_completion.d

zsh completions have been installed to:
  /usr/local/share/zsh/site-functions
==> Summary
🍺  /usr/local/Cellar/argo/2.10.1_1: 8 files, 62.3MB
Removing: /usr/local/Cellar/argo/2.10.0... (6 files, 62.1MB)
Removing: /Users/acollins8/Library/Caches/Homebrew/argo--2.10.0.catalina.bottle.tar.gz... (25.8MB)
[acollins8@intuitdepe1a95 argo (⎈ |k3s-default:argo) (master)]$ argo version
argo: v2.3.0-rc3
  BuildDate: 2020-09-03T03:48:23Z
  GitCommit: 24c778388a56792e847fcc30bd92a10299451959
  GitTreeState: clean
  GitTag: v2.3.0-rc3
  GoVersion: go1.13.4
  Compiler: gc
  Platform: darwin/amd64

24c778388a56792e847fcc30bd92a10299451959 is actually master and this is very risky and urgently needs to be fixe.

Marking as P1 regression.

[alexec]

Cancel the panic:

/usr/local/bin/argo version
argo: latest+854444e.dirty
  BuildDate: 2020-09-03T04:18:43Z
  GitCommit: 854444e47ac00d146cb83d174049bfbb2066bfb2
  GitTreeState: dirty
  GitTag: v2.10.1
  GoVersion: go1.15
  Compiler: gc
  Platform: darwin/amd64