Skip to content

Conversation

@JeyJeyGao
Copy link
Contributor

@JeyJeyGao JeyJeyGao commented Jan 17, 2024

Resolves #867
Signed-off-by: Junjie Gao junjiegao@microsoft.com

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (ee72394) 65.14% compared to head (b2e8418) 64.93%.

Files Patch % Lines
cmd/notation/plugin/install.go 25.00% 4 Missing and 2 partials ⚠️
cmd/notation/plugin/list.go 0.00% 5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #870      +/-   ##
==========================================
- Coverage   65.14%   64.93%   -0.22%     
==========================================
  Files          45       45              
  Lines        2717     2729      +12     
==========================================
+ Hits         1770     1772       +2     
- Misses        787      795       +8     
- Partials      160      162       +2     

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

shizhMSFT
shizhMSFT previously approved these changes Jan 18, 2024
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Blocking the merge as I'm building from this branch and testing the new error messages locally.

@ghost
Copy link

ghost commented Jan 18, 2024

When running command on a malformed plugin executable file on Windows:
./notation.exe plugin install --file ./malformed-plugin/azure-kv/notation-azure-kv.exe, the command returns following error message:

Error: plugin installation failed: failed to get metadata of new plugin: failed to execute the get-plugin-metadata command for plugin azure-kv: fork/exec ./malformed-plugin/azure-kv/notation-azure-kv.exe: This version of %1 is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher..
Please ensure the plugin is executable and meet the installation requirements on the windows/amd64.

On linux:

Error: plugin installation failed: failed to get metadata of new plugin: failed to execute the get-plugin-metadata command for plugin azure-kv: fork/exec test-plugin/malformed-plugin/azure-kv/notation-azure-kv: exec format error.
Please ensure the plugin is executable and meet the installation requirements on the linux/amd64.

These error messages are quite verbose though, can we improve them? (contents starting with fork/exec are already in the DEBUG log)

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
@JeyJeyGao
Copy link
Contributor Author

When running command on a malformed plugin executable file on Windows: ./notation.exe plugin install --file ./malformed-plugin/azure-kv/notation-azure-kv.exe, the command returns following error message:

Error: plugin installation failed: failed to get metadata of new plugin: failed to execute the get-plugin-metadata command for plugin azure-kv: fork/exec ./malformed-plugin/azure-kv/notation-azure-kv.exe: This version of %1 is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher..
Please ensure the plugin is executable and meet the installation requirements on the windows/amd64.

On linux:

Error: plugin installation failed: failed to get metadata of new plugin: failed to execute the get-plugin-metadata command for plugin azure-kv: fork/exec test-plugin/malformed-plugin/azure-kv/notation-azure-kv: exec format error.
Please ensure the plugin is executable and meet the installation requirements on the linux/amd64.

These error messages are quite verbose though, can we improve them? (contents starting with fork/exec are already in the DEBUG log)

Updated to:

./bin/notation plugin install --file ~/download/notation-azure-kv_1.0.1_darwin_amd64.tar.gz -d
DEBU[2024-01-18T12:55:26+08:00] Extracting file notation-azure-kv...         
DEBU[2024-01-18T12:55:26+08:00] Extracting file LICENSE...                   
DEBU[2024-01-18T12:55:26+08:00] Installing plugin from plugin path /tmp/notation-plugin2182206927 
DEBU[2024-01-18T12:55:26+08:00] Plugin get-plugin-metadata request: {}       
ERRO[2024-01-18T12:55:26+08:00] plugin get-plugin-metadata execution status: fork/exec /tmp/notation-plugin2182206927/notation-azure-kv: exec format error 
Error: plugin installation failed: failed to get metadata of new plugin: failed to execute the get-plugin-metadata command for plugin azure-kv.
Please ensure the plugin is executable and meet the installation requirements on the linux/amd64.

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
ghost
ghost previously approved these changes Jan 18, 2024
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost dismissed their stale review January 18, 2024 08:12

pending on dependency PR

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost merged commit dc786bc into notaryproject:main Jan 22, 2024
@ghost ghost mentioned this pull request Jan 25, 2024
6 tasks
rgnote pushed a commit to rgnote/notation that referenced this pull request Mar 8, 2024
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
rgnote pushed a commit to rgnote/notation that referenced this pull request Mar 8, 2024
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
Signed-off-by: rgnote <5878554+rgnote@users.noreply.github.com>
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
FeynmanZhou pushed a commit to FeynmanZhou/notation that referenced this pull request May 15, 2025
Signed-off-by: Junjie Gao <junjiegao@microsoft.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve notation plugin error message

5 participants