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

chore!: remove built-in binaries for forc and fuel-core #2143

Open
wants to merge 123 commits into
base: master
Choose a base branch
from

Conversation

petertonysmith94
Copy link
Contributor

@petertonysmith94 petertonysmith94 commented Apr 23, 2024

Closes #2084
Closes #2277


Summary

  • Internalises the forc and fuel-core binaries (removed usage for end-consumer).
  • Adds the ability to pass in custom binary paths via forcPath and fuelCorePath to the fuels.config.ts.
    • Defaults to forc and fuel-core
  • Binary not found and version mismatches errors for fuels CLI.
  • Suppressed the following related logs (experienced in the CI):
/bin/sh: 1: fuel-core: not found
/bin/sh: 1: forc: not found

Breaking change

  1. Built-in binaries have been removed, so the following means of running forc or fuel-core are not available.
# Before
npx fuels forc
npx fuels-forc
# AND
npx fuels core
npx fuels-core

Suggested to install fuelup and associated binaries (forc and fuel-core).

# After
forc
fuel-core
  1. fuels.config - removed options useBuiltinForc and useBuiltinFuelCore

  2. launchNode - removed option for useSystemFuelCore


Scenarios Both instances will stop the process until the issue/s are resolved.
  1. Non-existent binaries
> [email protected] fuels:dev /fuel/0.0.0-pr-2143-20240513112950
> fuels dev

Unable to find the following binaries on the filesystem:
 -> 'forc' at path 'forc'
 -> 'fuel-core' at path 'fuel-core'

Visit https://docs.fuel.network/guides/installation/ for an installation guide.
  1. Version mismatch
> [email protected] fuels:dev /fuel/0.0.0-pr-2143-20240513112950
> fuels dev

The following binaries on the filesystem are outdated:
 -> 'forc' is currently '0.55.0', but requires '0.56.1'.
 -> 'fuel-core' is currently '0.23.0', but requires '0.26.0'.

nedsalk
nedsalk previously approved these changes Apr 23, 2024
@petertonysmith94 petertonysmith94 marked this pull request as draft April 23, 2024 12:46
@petertonysmith94
Copy link
Contributor Author

Converted to draft to implement the changes as proposed here and here.

@petertonysmith94 petertonysmith94 dismissed nedsalk’s stale review April 23, 2024 12:48

Furthering the scope of this review, so dismissing review :)

@nedsalk
Copy link
Contributor

nedsalk commented May 15, 2024

Suppressed the following related logs (experienced in the CI):

/bin/sh: 1: fuel-core: not found
/bin/sh: 1: forc: not found

Why would you suppress these logs? Do you know the underlying cause of them?

@petertonysmith94
Copy link
Contributor Author

petertonysmith94 commented May 15, 2024

Suppressed the following related logs (experienced in the CI):

/bin/sh: 1: fuel-core: not found
/bin/sh: 1: forc: not found

Why would you suppress these logs? Do you know the underlying cause of them?

Turns out they were not just outputted to our CI, and were being displayed to the end-user when running npx fuels versions or similar.

It was introduced here, and appears to be when the command errors we'd pipe this to stderr - we now ignore this.

Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

Good stuff! 🚀

apps/demo-fuels/fuels.config.minimal.ts Outdated Show resolved Hide resolved
packages/account/src/test-utils/launchNode.ts Outdated Show resolved Hide resolved
packages/fuels/src/cli/commands/build/buildSwayPrograms.ts Outdated Show resolved Hide resolved
packages/utils/src/cli-utils/tryFindBinaries.ts Outdated Show resolved Hide resolved
packages/utils/src/cli-utils/tryFindBinaries.ts Outdated Show resolved Hide resolved
packages/utils/src/cli-utils/tryFindBinaries.ts Outdated Show resolved Hide resolved
packages/utils/src/cli-utils/tryFindBinaries.ts Outdated Show resolved Hide resolved
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.73%(-0.05%) 70.64%(-0.09%) 76.82%(+0%) 79.81%(-0.05%)
Changed Files:
Ok File (✨=New File) Lines Branches Functions Statements
🔴 packages/account/src/test-utils/launchNode.ts 100%
(+0%)
85.18%
(-3.28%)
68.42%
(+0%)
94.23%
(-0.05%)
🔴 ✨ packages/create-fuels/scripts/rewriteTemplateFiles.ts 0%
(+0%)
100%
(+100%)
100%
(+100%)
0%
(+0%)
✨ packages/fuels/src/cli/commands/withBinaryPaths.ts 100%
(+100%)
100%
(+100%)
100%
(+100%)
100%
(+100%)
🔴 ✨ packages/utils/src/cli-utils/tryFindBinaries.ts 100%
(+100%)
76.47%
(+76.47%)
100%
(+100%)
100%
(+100%)

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

Successfully merging this pull request may close these issues.

Handle system<>SDK binary version mismatch for fuels CLI Remove built-in binaries
8 participants