Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(cli): update permission prompt message for compiled binaries #24081

Conversation

yazan-abdalrahman
Copy link
Contributor

When running a compiled Deno program that requires permissions, the prompt message previously instructed users to rerun with --allow-* flags, which is incorrect for binaries.

Changes:

  • Updated the permission prompt message to instruct users to specify required permissions during the compilation step using deno compile --allow-*.
  • Adjusted PermissionDenied error messages to reflect the correct instructions for binaries.

This fix ensures that users are correctly informed about how to handle permissions for compiled Deno binaries.

Resolves #23250

When running a compiled Deno program that requires permissions, the prompt message previously instructed users to rerun with `--allow-*` flags, which is incorrect for binaries.

Changes:
- Updated the permission prompt message to instruct users to specify required permissions during the compilation step using `deno compile --allow-*`.
- Adjusted `PermissionDenied` error messages to reflect the correct instructions for binaries.

This fix ensures that users are correctly informed about how to handle permissions for compiled Deno binaries.

Resolves denoland#23250
@CLAassistant
Copy link

CLAassistant commented Jun 2, 2024

CLA assistant check
All committers have signed the CLA.

@yazan-abdalrahman yazan-abdalrahman changed the title Fix: Update permission prompt message for compiled binaries fix(cli): Update permission prompt message for compiled binaries Jun 2, 2024
@yazan-abdalrahman yazan-abdalrahman changed the title fix(cli): Update permission prompt message for compiled binaries fix(cli): update permission prompt message for compiled binaries Jun 3, 2024
@lucacasonato
Copy link
Member

Did you write this PR yourself, or is it AI generated? The addition of global_flags crate seems somewhat unnecessary?

@yazan-abdalrahman
Copy link
Contributor Author

Did you write this PR yourself, or is it AI generated? The addition of global_flags crate seems somewhat unnecessary?

Hi @lucacasonato

I wrote it,
I searched about dependencies to help me pass a flag to indicate that I am running from binary and not uniray. I found this is_unary param in the prompt method; it was hardcoded with a value of true; it was not passed along with the flow, so it is difficult to update the flow to pass it, so we need a flag to indicate we are running from binary standalone by passing the flag. I believe this idea is better.

Do you have a better way?

…-to-indicate-compile-time-permission-specification' into Update-permission-prompt-message-to-indicate-compile-time-permission-specification
@yazan-abdalrahman
Copy link
Contributor Author

Hello @bartlomieju
, could you please review the latest changes?

@yazan-abdalrahman
Copy link
Contributor Author

@bartlomieju Can we merge it?

@bartlomieju bartlomieju added this to the 1.46 milestone Jul 18, 2024
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

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

LGTM

@dsherret dsherret enabled auto-merge (squash) August 20, 2024 00:46
@dsherret dsherret merged commit 4f49f70 into denoland:main Aug 20, 2024
17 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading recommendation for permissions when using compiled binary
5 participants