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.

Sign up for GitHub

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

Jump to bottom

improve error message when package.xml is malformed #598

Closed
wjwwood opened this issue Mar 19, 2018 · 4 comments
Closed

improve error message when package.xml is malformed #598

wjwwood opened this issue Mar 19, 2018 · 4 comments
Labels
enhancement help wanted
Milestone
Untargeted

Comments

@wjwwood
Copy link
Contributor

wjwwood commented Mar 19, 2018

Anytime there's an issue with a package.xml you get something like reported in #597 (comment).

Instead it should avoid the traceback and the message to report it on the rosdep repository. Also, it should include which package.xml has the issue.
@wjwwood wjwwood added this to the Untargeted milestone Mar 19, 2018
@Nickolaim
Copy link
Contributor

I am happy to work on this but I need a repro that I can do on a clean machine. I suspect in this case it should include invalid package.xml and a command line.

@wjwwood
Copy link
Contributor Author

wjwwood commented Mar 20, 2018

I think you could take any workspace (like with just catkin in it) and modify the package.xml to have :{version} in the <version> tag. I'm not sure about the other failure modes. Might need to read the logic in catkin_pkg to see what errors can be thrown.

@Nickolaim
Copy link
Contributor

So here is my plan:

  1. Update catkin_pkg repo, so that the error message for InvalidPackage exception contains the file name if it is available
  2. Update the main exception handler in rosdep so it treats InvalidPackage specially - just print the error message and exits (no traceback, no standard exception message)
  3. Add tests to both repos

How does it sound?

Nickolaim referenced this issue in Nickolaim/catkin_pkg Mar 22, 2018
parse_package_string() raises InvalidPackage exception with the p…
b27c0fd 
…ackage name in the message.

This work is needed for [https://github.com/ros-infrastructure/rosdep/issues/598](https://github.com/ros-infrastructure/rosdep/issues/598)
This was referenced Mar 22, 2018
Merged
Closed
dirk-thomas referenced this issue in ros-infrastructure/catkin_pkg Apr 4, 2018
@Nickolaim @dirk-thomas
InvalidPackage with package path (#209)
ee7fc82 
* `parse_package_string()` raises `InvalidPackage` exception with the package name in the message.
This work is needed for [https://github.com/ros-infrastructure/rosdep/issues/598](https://github.com/ros-infrastructure/rosdep/issues/598)

* Add filename to the InvalidPackage exception coming from `Package.validate()`

* `InvalidPackage` has `msg` and`package_path` (#1)

`InvalidPackage` exception has `msg`, `package_path` and defines `__str__()`

Note that `parse_package_string()` catches `InvalidPackage` exception and sets
the filename; it is needed because the functions that called in
`_parse_package_string()` raise InvalidPackage exception without filename,
e.g. `_get_node()`

* Minor code review fixes
- Use double quotes for string literals with single quotes inside
- Use forward slash instead of backward slash in file path

* Pass filename to functions that raise InvalidPackage

Remove not used `_get_optional_node_value()`
Remove try/except block that was adding filename to InvalidPackage
exception

* nipick: improve error message
@Nickolaim Nickolaim mentioned this issue Apr 29, 2018
Closed
Nickolaim referenced this issue in Nickolaim/catkin_pkg May 3, 2018
@Nickolaim
InvalidPackage with package path (ros-infrastructure#209)
de67115 
* `parse_package_string()` raises `InvalidPackage` exception with the package name in the message.
This work is needed for [https://github.com/ros-infrastructure/rosdep/issues/598](https://github.com/ros-infrastructure/rosdep/issues/598)

* Add filename to the InvalidPackage exception coming from `Package.validate()`

* `InvalidPackage` has `msg` and`package_path` (#1)

`InvalidPackage` exception has `msg`, `package_path` and defines `__str__()`

Note that `parse_package_string()` catches `InvalidPackage` exception and sets
the filename; it is needed because the functions that called in
`_parse_package_string()` raise InvalidPackage exception without filename,
e.g. `_get_node()`

* Minor code review fixes
- Use double quotes for string literals with single quotes inside
- Use forward slash instead of backward slash in file path

* Pass filename to functions that raise InvalidPackage

Remove not used `_get_optional_node_value()`
Remove try/except block that was adding filename to InvalidPackage
exception

* nipick: improve error message
dirk-thomas referenced this issue in ros-infrastructure/catkin_pkg May 3, 2018
@Nickolaim @dirk-thomas
Flake8, #210 (#211)
ee9d583 
* Enable flake8 for the project as a unit test. The test is copied from [https://github.com/ros-infrastructure/rosdep/blob/d31c5320b95f4e9ff6a0aad56a8531cb138fc079/test/test_flake8.py](https://github.com/ros-infrastructure/rosdep/blob/d31c5320b95f4e9ff6a0aad56a8531cb138fc079/test/test_flake8.py)
Code re-formatted, fixed other issues raised by flake8

* Do not run flake8 test on Python 3.2
Add Python 3.6 to Travis CI

* keep operator trailing

* avoid aligned indentation

* avoid aligned indentation

* remove unnecessary parenthesis

* avoid aligned indentation

* remove old comment

* conf.py doesn't exist

* remove usage of format_map which is not available in Python 2.7

* remove usage of format_map which is not available in Python 2.7

* Include package name in error message (#212)

* Include package name in error message

* Wrapped package name in quotes for consistency

* Add support for file attribution of license tag (#202)

* InvalidPackage with package path (#209)

* `parse_package_string()` raises `InvalidPackage` exception with the package name in the message.
This work is needed for [https://github.com/ros-infrastructure/rosdep/issues/598](https://github.com/ros-infrastructure/rosdep/issues/598)

* Add filename to the InvalidPackage exception coming from `Package.validate()`

* `InvalidPackage` has `msg` and`package_path` (#1)

`InvalidPackage` exception has `msg`, `package_path` and defines `__str__()`

Note that `parse_package_string()` catches `InvalidPackage` exception and sets
the filename; it is needed because the functions that called in
`_parse_package_string()` raise InvalidPackage exception without filename,
e.g. `_get_node()`

* Minor code review fixes
- Use double quotes for string literals with single quotes inside
- Use forward slash instead of backward slash in file path

* Pass filename to functions that raise InvalidPackage

Remove not used `_get_optional_node_value()`
Remove try/except block that was adding filename to InvalidPackage
exception

* nipick: improve error message

* Fix flake8 issues (1 empty line instead of 2) for recently updated code
@Nickolaim Nickolaim mentioned this issue May 19, 2018
Merged
@Nickolaim
Copy link
Contributor

All changes are in, I propose to close the issue.

@wjwwood wjwwood closed this as completed May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement help wanted
Projects
None yet
Milestone
Untargeted
Development

No branches or pull requests
2 participants
@wjwwood @Nickolaim