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

There's no way to NOT use install space #176

Closed
130s opened this issue May 15, 2017 · 2 comments
Closed

There's no way to NOT use install space #176

130s opened this issue May 15, 2017 · 2 comments
Labels
enhancement

Comments

@130s
Copy link
Member

130s commented May 15, 2017

Opening as a separate ticket of what I poseted in #150 (review).

Because catkin config --install is always run:

  • there seems to be no way to NOT build into install space.
  • If I'm right I want to add a feature to allow disabling install space.

Am I wrong?

This can cause an issue like ros-industrial-consortium/descartes#198 (comment) where users want to test with the resources that are not deployed.
(For his case, I thought NOT_TEST_INSTALL=true could have worked but I was wrong; NOT_TEST_INSTALL just avoids running tests during the test steps for the install space.)
@mathias-luedtke
Copy link
Member

there seems to be no way to NOT build into install space.

For now BEFORE_SCRIPT='catkin config -w $CATKIN_WORKSPACE --no-install' is the way to go.

There are a couple of ways to make this more user-friendly:

  1. remove catkin config --install, this would break the behaviour again, so 👎
  2. suppress catkin config --install with an opt-out flag like USE_DEVEL_SPACE_ONLY
  3. catkin config ${CATKIN_CONFIG:---install}
  4. catkin config --install followed by if [ -n "$CATKIN_CONFIG" ]; then catkin config $CATKIN_CONFIG; fi
  5. suppressable catkin config --install followed by if [ -n "$CATKIN_CONFIG" ]; then catkin config $CATKIN_CONFIG; fi

I'd prefer option 4, as a CATKIN_CONFIG could be beneficial for other use cases.
Option 5 is okay, but I'd like to avoid yet another specific flag.

@mathias-luedtke mathias-luedtke mentioned this issue May 15, 2017
Closed
130s added a commit to 130s/industrial_ci that referenced this issue May 15, 2017
@130s
[WIP] Allow passing 'catkin config' parameters (addresses ros-industr…
9848597 
…ial#176).

Issue discussed in ros-industrial#176

- [ ] Unit test; Is `industrial_ci/mockups/industrial_ci_testpkg` not ready (I see `CATKIN_IGNORE` is placed in it)?
- [ ] Documentation.
@130s 130s mentioned this issue May 15, 2017
Merged
2 tasks
130s added a commit to 130s/industrial_ci that referenced this issue May 16, 2017
@130s
[WIP] Allow passing 'catkin config' parameters (addresses ros-industr…
e28a4f8 
…ial#176).

Issue discussed in ros-industrial#176

- [ ] Unit test; Is `industrial_ci/mockups/industrial_ci_testpkg` not ready (I see `CATKIN_IGNORE` is placed in it)?
- [ ] Documentation.
130s added a commit to 130s/industrial_ci that referenced this issue May 16, 2017
@130s
[WIP] Allow passing 'catkin config' parameters (addresses ros-industr…
6f6bb85 
…ial#176).

Issue discussed in ros-industrial#176

- [ ] Unit test; Is `industrial_ci/mockups/industrial_ci_testpkg` not ready (I see `CATKIN_IGNORE` is placed in it)?
- [ ] Documentation.
130s added a commit to 130s/industrial_ci that referenced this issue May 23, 2017
@130s
[WIP] Allow passing 'catkin config' parameters (addresses ros-industr…
94cd998 
…ial#176).

Issue discussed in ros-industrial#176

- [ ] Unit test; Is `industrial_ci/mockups/industrial_ci_testpkg` not ready (I see `CATKIN_IGNORE` is placed in it)?
- [ ] Documentation.
130s added a commit to 130s/industrial_ci that referenced this issue May 24, 2017
@130s
Allow passing 'catkin config' parameters (addresses ros-industrial#176).
a4ad9fb 
Add a mockup package that installs limited resource (primarily for ros-industrial#177).
'catkin config --install' enforcement as a default (see rationale at ros-industrial#177 (comment)).
[doc] Add CATKIN_CONFIG.
130s added a commit that referenced this issue May 24, 2017
@130s
Allow passing 'catkin config' parameters (addresses #176).
1306c2d 
Add a mockup package that installs limited resource (primarily for #177).
'catkin config --install' enforcement as a default (see rationale at #177 (comment)).
[doc] Add CATKIN_CONFIG.
@130s
Copy link
Member Author

130s commented May 24, 2017

Addressed in #177

@130s 130s closed this as completed May 24, 2017
130s added a commit to mathias-luedtke/industrial_ci that referenced this issue Jun 9, 2017
@130s
Allow passing 'catkin config' parameters (addresses ros-industrial#176).
3a55507 
Add a mockup package that installs limited resource (primarily for ros-industrial#177).
'catkin config --install' enforcement as a default (see rationale at ros-industrial#177 (comment)).
[doc] Add CATKIN_CONFIG.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mathias-luedtke @130s