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

Option of using a command-line paramter within a condition attribute of dependency tag #104

Open
sumanth-nirmal opened this issue May 27, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@sumanth-nirmal
Copy link

sumanth-nirmal commented May 27, 2020

For now, we can only use env-variables within a expression of condition attribute for a dependency tag in Package.xml as below:

<depend condition="$ROS_VERSION == 1">roscpp</depend>

Is it possible to have a feature where we can pass in this variable as a command-line parameter instead of an environment variable?

Also, let me know if this is not the correct repo for this issue!

@dirk-thomas dirk-thomas added the question Further information is requested label May 27, 2020
@dirk-thomas
Copy link
Member

let me know if this is not the correct repo for this issue!

The behavior is described in the ROS REP 149.

Is it possible to have a feature where we can pass in this variable as a command-line parameter instead of an environment variable?

Maybe I don't understand what exactly you are asking for. What is the difference between a CLI parameter (like <cmd> --condition KEY=VALUE) vs. prefixing the same command to set an environment variable specifically for that invocation (like KEY=VALUE <cmd>)?

Imo the latter is clearly preferred since it works out-of-the-box while the former needs custom support from the CLI.

@jpsamper2009
Copy link

@dirk-thomas I think one advantage of having the command-line option is that it could be added as part of a mixin.

The use-case is that we have #ifndef FOO in our code, and we want to skip certain dependencies when FOO is set. For configuration foo, we have a mixin, colcon build --mixin foo which already sets --cmake-args -DFOO=1 (among other options), and it would be nice if the same mixin could also skip the dependencies that we want to skip. Does that make more sense?

cc @sumanth-nirmal

@sumanth-nirmal
Copy link
Author

@dirk-thomas I guess @jpsamper2009 answered with the exact use-case we have. Having an option to set the condition variable via mixin will get rid of setting the same named variable again to skip the dependencies.

FOO=1 colcon build --mixin foo

vs

colcon build --mixin foo

@jpsamper2009
Copy link

@dirk-thomas Any thoughts on the feature/use-case? Maybe we don't even need a command-line option in colcon, but rather a way to set environment variables in the .mixin file.

@dirk-thomas
Copy link
Member

Maybe we don't even need a command-line option in colcon, but rather a way to set environment variables in the .mixin file.

That sounds like the better approach to me.

I am not sure how to add it into the current yaml format without a collision though. Also I am not sure if the mixin is being processed early enough so that changing the environment takes effect for what you want to use it for - parse the manifest information.

@dirk-thomas dirk-thomas added enhancement New feature or request and removed question Further information is requested labels Jun 10, 2020
@jpsamper2009
Copy link

@dirk-thomas My guess is that we would need something like a --package-env FOO=1 option, no? That would be part of colcon-package-selection. And then this environment variable would need to get passed does to all the PackageSelectionExtensions? What plugin is taking care of parsing package.xml files?

@dirk-thomas
Copy link
Member

My guess is that we would need something like a --package-env FOO=1 option, no?
And then this environment variable would need to get passed does to all the PackageSelectionExtensions?

Based on the previous two comments I thought the direction would be to specify the information about the environment in the mixin - but not as a regular command line argument - but as a separate kind of information - since colcon doesn't have an argument like --package-env.

The environment modification would be applied to os.environ and work the same way as if colcon was invoked with that custom environment: FOO=var colcon <verb>.

That would be part of colcon-package-selection. What plugin is taking care of parsing package.xml files?

The parsing of the manifest files happens during package identification (https://colcon.readthedocs.io/en/released/developer/extension-point.html#packageidentificationextensionpoint) which occurs before the package selection. The actual parsing functionality is provided by catkin_pkg which is invoked from

pkg = parse_package(path)
.

@jpsamper2009
Copy link

@dirk-thomas Sorry for the confusion, when you said, "I am not sure if the mixin is being processed early enough so that changing the environment takes effect for what you want to use it for - parse the manifest information." I assumed you were saying that the mixin option was not going to work, so I was thinking it could make sense as part of the package-selection step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

3 participants