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

Update missing worker binaries error #2853

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

mrcnski
Copy link
Contributor

@mrcnski mrcnski commented Jan 4, 2024

Please let me know if this would be a better UX. Should we have specific links in the error message?

@mrcnski mrcnski added the T0-node This PR/Issue is related to the topic “node”. label Jan 4, 2024
@mrcnski mrcnski requested review from bkchr and ggwpez January 4, 2024 11:40
@mrcnski mrcnski self-assigned this Jan 4, 2024
@@ -239,7 +239,7 @@ pub enum Error {
InvalidWorkerBinaries { prep_worker_path: PathBuf, exec_worker_path: PathBuf },

#[cfg(feature = "full-node")]
#[error("Worker binaries could not be found, make sure polkadot was built/installed correctly. If you ran with `cargo run`, please run `cargo build` first. Searched given workers path ({given_workers_path:?}), polkadot binary path ({current_exe_path:?}), and lib path (/usr/lib/polkadot), workers names: {workers_names:?}")]
#[error("Worker binaries could not be found, make sure polkadot was built and installed correctly. Please see the readme or wiki for the latest instructions. If you ran with `cargo run`, please run `cargo build` first. Searched given workers path ({given_workers_path:?}), polkadot binary path ({current_exe_path:?}), and lib path (/usr/lib/polkadot), workers names: {workers_names:?}")]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe make a case-decision that describes it for local builds and for downloaded distribution paths?
Like: if you have the source code locally, run cargo build
if you downloaded it, then also download the two other bins and mark them as executable.

But yea, linking to some readme maybe good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean in the error message? Having too much specific info seems like it will be hard to keep up-to-date. Maybe just link to the readme - let me know if that's better.

And did you have to mark them executable? On Mac I just have to accept a pop-up that asks if they are safe to run - should this step be added to the readme?

Copy link
Member

Choose a reason for hiding this comment

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

I mean the chmod +x, otherwise there is an error that they are not executable. I guess its normal when downloading a binary 🤷‍♂️

@ggwpez ggwpez added the R0-silent Changes should not be mentioned in any release notes label Jan 4, 2024
@mrcnski mrcnski requested review from a team as code owners January 4, 2024 16:29
@ggwpez ggwpez merged commit e07476e into master Jan 4, 2024
121 of 122 checks passed
@ggwpez ggwpez deleted the mrcnski/missing-worker-binaries-error branch January 4, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T0-node This PR/Issue is related to the topic “node”.
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

3 participants