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

🌱 remove OS and Arch info from scorecard release binary name #4520

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

timothysparg
Copy link
Contributor

@timothysparg timothysparg commented Feb 11, 2025

What kind of change does this PR introduce?

release config

What is the current behavior?

  • scorecard_5.0.0_darwin_amd64
  • scorecard_5.0.0_darwin_arm64
  • scorecard_5.0.0_linux_amd64
  • scorecard_5.0.0_linux_arm64
  • scorecard_5.0.0_windows_amd64.exe
  • scorecard_5.0.0_windows_arm64.exe

What is the new behavior (if this is a feature change)?**

There will be a single binary called scorecard, or for windows scorecard.exe

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #4517

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Scorecard binary name is now consistent across all release platforms

Previously binaries were created with their architecture and OS included as part of their binary name.

Removing the `binary: scorecard-<linux|darwin|windows>-{{ .Arch }}` line allows us to collapse all of the different build configs into a single universal build that caters for [linux,darwin,windos]*[arm64,amd64]

`- -buildmode=exe` was not needed on the windows builds and was also removed.

closes ossf#4517

Signed-off-by: Tim Sparg <[email protected]>
As per https://goreleaser.com/deprecations#snapshotname_template `snapshot.name_template` has been replaced with `snapshot.version_template`

Signed-off-by: Tim Sparg <[email protected]>
@timothysparg timothysparg requested a review from a team as a code owner February 11, 2025 22:11
@timothysparg timothysparg requested review from justaugustus and spencerschrock and removed request for a team February 11, 2025 22:11
@timothysparg
Copy link
Contributor Author

After change

  • building binaries
    • building                                       binary=dist/universal_windows_arm64_v8.0/scorecard.exe
    • building                                       binary=dist/universal_linux_amd64_v1/scorecard
    • building                                       binary=dist/universal_linux_arm64_v8.0/scorecard
    • building                                       binary=dist/universal_windows_amd64_v1/scorecard.exe
    • building                                       binary=dist/universal_darwin_arm64_v8.0/scorecard
    • building                                       binary=dist/universal_darwin_amd64_v1/scorecard
    • took: 1m2s
  • archives
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_darwin_arm64.tar.gz
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_windows_amd64.tar.gz
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_linux_arm64.tar.gz
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_darwin_amd64.tar.gz
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_windows_arm64.tar.gz
    • archiving                                      name=dist/scorecard_SNAPSHOT-7534049e_linux_amd64.tar.gz
tar tvf dist/scorecard_SNAPSHOT-7534049e_darwin_amd64.tar.gz
-rw-r--r--  0 timsparg staff   11355 Feb 11 13:16 LICENSE
-rw-r--r--  0 timsparg staff   44586 Feb 11 13:16 README.md
-rwxr-xr-x  0 timsparg staff 54996608 Feb 11 22:53 scorecard    

@spencerschrock
Copy link
Member

spencerschrock commented Feb 11, 2025

- -buildmode=exe was not needed on the windows builds and was also removed.

I'm not well versed in that option, can you elaborate on this change? I'm not saying we need it, just trying to understand.

It slightly changes the size of windows_amd64 for example. And changes the permissions of all binaries from rwxr-xr-x (without buildmode=exe) to rwxr-x--- (with buildmode=exe).

-buildmode=default
Listed main packages are built into executables and listed
non-main packages are built into .a files (the default
behavior).

-buildmode=exe
Build the listed main packages and everything they import into
executables. Packages not named main are ignored.

It seems like default is usually exe, unless you're on Darwin or Windows in which case it's PIE?
https://github.com/golang/go/blob/b941d2b6d8bd9663abec7761de366b09a2be7445/src/cmd/go/internal/work/init.go#L231-L238
https://github.com/golang/go/blob/b941d2b6d8bd9663abec7761de366b09a2be7445/src/internal/platform/supported.go#L238-L253

@spencerschrock
Copy link
Member

Although in both cases (with and without specifying the flag) I see the binary is build with buildmode=exe:

go version -m scorecard.exe  | grep buildmode
        build   -buildmode=exe

@timothysparg
Copy link
Contributor Author

Although in both cases (with and without specifying the flag) I see the binary is build with buildmode=exe:

go version -m scorecard.exe  | grep buildmode
        build   -buildmode=exe

I could claim omniscience and say I expected it to do that, but really I'm just left scratching my head.

I tried to get a better understanding of what was going on with the different buildmodes, and I now just feel like I'm missing something.

current goreleaser output (default buildmode)

Steps to recreate

  • used timothysparg:feat/scorecard-binary as the base
  • reverted to the old goreleaser config (windows with -buildmode=exe)
  • updated thename_template deprecation (otherwise make build-releaser exits prematurely)

Expectation

  • pie
    • Darwin
  • exe
    • LInux
    • Windows

Output

PLATFORM ARCH FILENAME RIGHTS SIZE BUILDMODE
darwin amd64 scorecard-darwin-amd64 -rwxr-xr-x 53MiB (53707KB) exe
darwin arm64 scorecard-darwin-arm64 -rwxr-xr-x 51MiB (52175KB) exe
linux amd64 scorecard-linux-amd64 -rwxr-xr-x 51MiB (52000KB) exe
linux arm64 scorecard-linux-arm64 -rwxr-xr-x 49MiB (50112KB) exe
windows amd64 scorecard-windows-amd64.exe -rwxr-xr-x 52MiB (52856KB) exe
windows arm64 scorecard-windows-arm64.exe -rwxr-xr-x 50MiB (50913KB) exe

Updated goreleaser output

Steps to recreate

  • used timothysparg:feat/scorecard-binary

Expectation

  • pie
    • Darwin
    • Windows
  • exe
    • LInux

Output

PLATFORM ARCH FILENAME RIGHTS SIZE BUILDMODE
darwin amd64 scorecard -rwxr-xr-x 53MiB (53707KB) exe
darwin arm64 scorecard -rwxr-xr-x 51MiB (52175KB) exe
linux amd64 scorecard -rwxr-xr-x 51MiB (52000KB) exe
linux arm64 scorecard -rwxr-xr-x 49MiB (50112KB) exe
windows amd64 scorecard.exe -rwxr-xr-x 53MiB (53466KB) exe
windows arm64 scorecard.exe -rwxr-xr-x 50MiB (50913KB) exe

Updated goreleaser output using pie buildmode

Steps to recreate

  • used timothysparg:feat/scorecard-binary
  • added -buildmode=pie

Expectation

  • pie
    • Darwin
    • Windows
  • exe
    • LInux

Output

PLATFORM ARCH FILENAME RIGHTS SIZE BUILDMODE
darwin amd64 scorecard -rwxr-xr-x 53MiB (53707KB) exe
darwin arm64 scorecard -rwxr-xr-x 51MiB (52175KB) exe
linux amd64 scorecard -rwxr-xr-x 58MiB (59076KB) exe
linux arm64 scorecard -rwxr-xr-x 56MiB (57216KB) exe
windows amd64 scorecard.exe -rwxr-xr-x 53MiB (53466KB) exe
windows arm64 scorecard.exe -rwxr-xr-x 50MiB (50913KB) exe

Duh moment

What I realised at this point is that the buildmode wasn't making any difference 🙄

I took a step back and had a looked at the goreleaser docs and saw that buildmode is a top level field (same level as id and flags etc).

- id: universal
  flags:
      - -trimpath
      - -tags=netgo
  mod_timestamp: '{{ .CommitTimestamp }}'
  goos:
    - linux
    - darwin
    - windows
  goarch:
    - amd64
    - arm64
  ldflags:
    - -s {{.Env.VERSION_LDFLAGS}} 
  buildmode: pie

given this configuration I now get the following output

PLATFORM ARCH FILENAME RIGHTS SIZE BUILDMODE
darwin amd64 scorecard -rwxr-xr-x 53MiB (53707KB) pie
darwin arm64 scorecard -rwxr-xr-x 51MiB (52175KB) pie
linux amd64 scorecard -rwxr-xr-x 58MiB (59084KB) pie
linux arm64 scorecard -rwxr-xr-x 56MiB (57216KB) pie
windows amd64 scorecard.exe -rwxr-xr-x 53MiB (53466KB) pie
windows arm64 scorecard.exe -rwxr-xr-x 50MiB (50913KB) pie

Thoughts

  • I don't think the -buildmode=exe flag was doing anything
  • I still don't understand why the defaults you mentioned above aren't being shown (I had a quick look at goreleaser and couldn't see that it was being opinionated)

Are there any reason we shouldn't just set the buildmode to pie?

@spencerschrock
Copy link
Member

spencerschrock commented Feb 12, 2025

What I realised at this point is that the buildmode wasn't making any difference

It is though, hence the size difference.
with defaults:

file scorecard.exe
scorecard.exe: PE32+ executable (console) x86-64, for MS Windows, 8 sections

with - -buildmode=exe flag

file scorecard.exe
scorecard.exe: PE32+ executable (console) x86-64, for MS Windows, 7 sections

The missing PE section is .reloc with a size of ~615KB (as determined by readpe), which seems equal to the difference in sizes. I'm assuming the .reloc section is needed for ASLR on windows.

Are there any reason we shouldn't just set the buildmode to pie?

It produces dynamic libraries instead of static on linux. Let's keep the default buildmode for now.

Copy link
Member

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thanks, appreciate the deduplication.

Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.46%. Comparing base (353ed60) to head (5429d42).
Report is 112 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4520      +/-   ##
==========================================
+ Coverage   66.80%   68.46%   +1.65%     
==========================================
  Files         230      246      +16     
  Lines       16602    18444    +1842     
==========================================
+ Hits        11091    12627    +1536     
- Misses       4808     4991     +183     
- Partials      703      826     +123     

@spencerschrock spencerschrock changed the title 🌱 remove OS and Arch info from scorecard binary 🌱 remove OS and Arch info from scorecard release binary name Feb 12, 2025
@spencerschrock spencerschrock merged commit 6fc296e into ossf:main Feb 12, 2025
43 checks passed
@timothysparg
Copy link
Contributor Author

@spencerschrock 🤦 I was fixated on why I wasn't seeing pie vs exe in the builder and completely stopped looking at the numbers... talk about missing the forest for the trees!

balteravishay pushed a commit to PrinceAsiedu/scorecard that referenced this pull request Feb 17, 2025
* feat: create binary without OS and arch in name

Previously binaries were created with their architecture and OS included as part of their binary name.

Removing the `binary: scorecard-<linux|darwin|windows>-{{ .Arch }}` line allows us to collapse all of the different build configs into a single universal build that caters for [linux,darwin,windos]*[arm64,amd64]

`- -buildmode=exe` was not needed on the windows builds and was also removed.

closes ossf#4517

Signed-off-by: Tim Sparg <[email protected]>

* fix: resolve name_template deprecation

As per https://goreleaser.com/deprecations#snapshotname_template `snapshot.name_template` has been replaced with `snapshot.version_template`

Signed-off-by: Tim Sparg <[email protected]>

---------

Signed-off-by: Tim Sparg <[email protected]>
Signed-off-by: balteravishay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

have a single binary name across all platforms
2 participants