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

Upgrade yarn to v3.8.5 #2459

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

andreasgerstmayr
Copy link
Contributor

@andreasgerstmayr andreasgerstmayr commented Sep 26, 2024

Which problem is this PR solving?

Description of the changes

  • Upgrade from yarn classic to yarn v3.8.5

The lint step is broken at the moment, because TypeScript incorrectly resolves the typings for react-router v5 with the types from react-router v6 (installed as a dependency of react-router-dom-v5-compat) instead of @types/react-router-dom. I'm not sure why this works correctly with yarn v1 though. See remix-run/react-router#9892

How was this change tested?

  • jaeger all-in-one & yarn start

Checklist

Copy link

codecov bot commented Sep 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.61%. Comparing base (3c88048) to head (2344b6b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2459   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files         254      254           
  Lines        7679     7679           
  Branches     1931     1995   +64     
=======================================
  Hits         7419     7419           
  Misses        260      260           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreasgerstmayr andreasgerstmayr changed the title [WIP] Uprade yarn to v3.8.5 [WIP] Upgrade yarn to v3.8.5 Sep 26, 2024
@pavolloffay pavolloffay added the changelog:dependencies Update to dependencies label Sep 26, 2024
Signed-off-by: Andreas Gerstmayr <[email protected]>
@andreasgerstmayr andreasgerstmayr changed the title [WIP] Upgrade yarn to v3.8.5 Upgrade yarn to v3.8.5 Sep 26, 2024
@andreasgerstmayr andreasgerstmayr marked this pull request as ready for review September 26, 2024 13:44
@andreasgerstmayr andreasgerstmayr requested a review from a team as a code owner September 26, 2024 13:44
@andreasgerstmayr andreasgerstmayr requested review from pavolloffay and removed request for a team September 26, 2024 13:44
Comment on lines +31 to +33
!.yarn/releases
!.yarn/sdks
!.yarn/versions
Copy link
Member

Choose a reason for hiding this comment

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

Why would we not ignore these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this list straight from the yarn docs here: https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
There's an explanation in that FAQ entry for each path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding checking in yarn sources, afaics we have two options when using yarn v2+:

  • use the experimental corepack feature
  • or commit the yarn sources

see yarnpkg/berry#5748 for more details

Copy link
Member

Choose a reason for hiding this comment

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

corepack seems preferable, I am strongly against vendoring yarn code in the repo (feels insane to me that they would even propose this as a default setup).

In the issue there were mentions of problems with how corepack operates depending on versions. Probably not an issue for jaeger-ui.

Q: why yarn 3.8 and not 4.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for yarn 3.8 was that we want to use cachi2 (used to ensure the build is hermetic, i.e. it caches dependencies to support building Jaeger UI without network access), which at the moment only supports yarn v3 (I assume v4 might also be supported, but I didn't test it).

Alternatively, we could also use npm. It also supports workspaces now (in case this was the reason to switch to yarn in the first place). What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I don't really follow. We have a yarn.lock file that ensures repeatable build. Are we worried about supply chain attack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's about supply chain attacks. Our downstream builds need to be hermetic according to SLSA and include a Software Bill of Materials (SBOM) etc.

You're right, effectively the yarn package manager already supports these properties. We need to support multiple languages and package managers though, so this part of the build pipeline is handled by an external tool like cachi2 for our downstream builds.

@andreasgerstmayr
Copy link
Contributor Author

Hm, somehow the s390x build is stuck because of the upgrade to yarn v3. I'll see with IBM if we can get this resolved.

@andreasgerstmayr andreasgerstmayr marked this pull request as draft September 26, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:dependencies Update to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants