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

Fix line number in error reporting during manifest validation: #614

Merged

Conversation

alshamams
Copy link
Contributor

The plug-in manifest is validated in two phases: i) when it is first loaded, ii) when the manifest is inline edited. In the former case, the validation is carried out in OSGI layers, and the exception caught in the pde modules with no bearing on the faulty line number, in case of issues.

This commit tries to parse the exception message in a best effort basis, extracts the header that is subjected for the fault and obtains the line number of the header. It falls back on line 1, in case of parse error.

Fixes: #613

@github-actions
Copy link

github-actions bot commented Jun 10, 2023

Test Results

   237 files   -        6     237 suites   - 6   51m 6s ⏱️ - 3m 13s
3 286 tests +       1  3 260 ✔️ ±       0  24 💤 ±  0  2 +2 
7 431 runs   - 2 721  7 381 ✔️  - 2 698  48 💤  - 24  2 +2 

For more details on these failures, see this check.

Results for commit adcbe18. ± Comparison against base commit 57007dd.

♻️ This comment has been updated with latest results.

Copy link
Member

@vik-chand vik-chand left a comment

Choose a reason for hiding this comment

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

  1. The variable names like t1,t2,hdr can be improved
  2. StringTokenizer is legacy, split should be preferred
  3. Also a short comment on what you are trying to achieve

@alshamams
Copy link
Contributor Author

In some cases manifest validation takes place in the OSGI layers without obtaining the erroneous line number, as is the case with bundle description validation, thereby displaying it on line 1. This may lead to confusion regarding line number.
This commit tries to parse the exception message, extracts the header from the error message and obtains the line number of the header. It falls back on line 1, in case of parse error.

Copy link
Member

@vik-chand vik-chand left a comment

Choose a reason for hiding this comment

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

The declaration at line 149 could be moved down. Update license year if required.
split could be renamed as splitArray. Also one liner comment on what you are doing.

@alshamams
Copy link
Contributor Author

Hi @vik-chand ,
Kindly look through the edits and let me know in case of any further changes.

Comment on lines +154 to +155
IHeader header = null;
header = getHeader(lastString);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
IHeader header = null;
header = getHeader(lastString);
IHeader header = getHeader(lastString);

@vik-chand
Copy link
Member

Please rebase wrt latest.

The plug-in manifest is validated in two phases: i) when it is first loaded,
ii) when the manifest is inline edited. In the former case, the validation
is carried out in OSGI layers, and the exception caught in the pde modules
with no bearing on the faulty line number, in case of issues.

This commit tries to parse the exception message in a best effort basis,
extracts the header that is subjected for the fault and obtains the line
number of the header. It falls back on line 1, in case of parse error.
@alshamams alshamams force-pushed the bundle_error_reporting_line branch from ced50ec to adcbe18 Compare July 3, 2023 12:07
@gireeshpunathil gireeshpunathil merged commit 385b549 into eclipse-pde:master Jul 3, 2023
12 of 14 checks passed
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.

Error marker being reported on incorrect line number during manifest validation
3 participants