Consider building to install space directly #54

130s · 2016-05-27T09:23:11Z

Consider building to install space directly. I don't see a reason why to build twice, for devel and install space. This only doubles the response time of the test.

The text was updated successfully, but these errors were encountered:

gavanderhoorn · 2016-05-27T19:10:57Z

This makes sense, I agree with @rhaschke.

130s · 2016-06-04T11:13:43Z

One usecase I can think of is that packages are not planned to be released so no need for install space either. In that case keeping to use devel space might be more convenient and less confusing because most of the tutorials use devel.

But I guess there may not be many users who really need to stick to devel space. So +1

gavanderhoorn · 2016-06-04T11:49:22Z

I seem to remember the 'official policy' is to recommend using install space as much as possible, especially with catkin_tools. The devel space is kept only for a measure of backwards compatibility.

Packages failing because they have no install(..) statements is a good thing I think: missing those will also cause problems when / if people try to release those packages through the build farm, so catching that early seems like a good thing to do.

rhaschke · 2016-06-04T22:11:50Z

Packages failing because they have no |install(..)| statements is a
good thing I think: missing those will also cause problems when / if
people try to release those packages through the build farm, so
catching that early seems like a good thing to do.

I can only underline this argument.

Omit entire catkin build section when devel space is not choosed to be used.

Omit entire catkin build section when devel space is not choosed to be used. Directory structure sorted out. Add another pkg solely for testing only. Conflicts: README.rst travis.sh [.travis.yml] Allow failure for now the jade source build (see ros-industrial/industrial_core#144 (comment)). [.travis.yml] Some more test matrix. [Travis] Rename a file to avoid confusion. Kinetic should better be no longer 'allow_failure'. warehouse_ros from shadow-fixed should pass. [Travis] Remove less meaningful job (warehouse_ros prerelease). More Kinetic jobs.

davetcoleman · 2016-07-07T15:08:55Z

+1 to install space by default

Omit entire catkin build section when devel space is not choosed to be used. Directory structure sorted out. Add another pkg solely for testing only. Conflicts: README.rst travis.sh [.travis.yml] Allow failure for now the jade source build (see ros-industrial/industrial_core#144 (comment)). [.travis.yml] Some more test matrix. [Travis] Rename a file to avoid confusion. Kinetic should better be no longer 'allow_failure'. warehouse_ros from shadow-fixed should pass. [Travis] Remove less meaningful job (warehouse_ros prerelease). More Kinetic jobs.

130s · 2016-09-12T21:59:03Z

(Hopefully) fixed by #67, where I accidentally (sorry...) included the commits that try to implement the fix to this.

130s · 2017-03-22T14:46:17Z

#54 (comment)

(Hopefully) fixed by #67, where I accidentally (sorry...) included the commits that try to implement the fix to this.

I don't think my statement above is correct; #67 doesn't add anything relevant.

I think #150 fixes this.

130s · 2017-03-26T14:52:45Z

Reopening for a discussion.

As a package with "industrial" prefix, requiring install space sounds reasonable. Users are pros, advanced grad students...

Meanwhile, when we want to escalate this package to general ROS level as we discussed (e.g. at #75 (comment)), we'll probably want to keep devel space usage. I can hardly imagine novice users and research students willing to use install space in their own software when they have no future plan to make an installable release. No?

mathias-luedtke · 2017-03-26T15:14:16Z

we'll probably want to keep devel space usage

No. The install option just runs the provided install rules on top of the build instructions.
So if you do not specify any rules, your code is not affected at all.
If you specify rules, they get tested.

I can hardly imagine novice users and research students willing to use install space in their own software when they have no future plan to make an installable release

They don't have to, but it is recommended anyway.
As soon as projects start growing you have to take care of dependencies and package relations.
If someone does not care about these topics, CI testing is not the way to go.

130s · 2017-03-26T16:16:05Z

Hm, I can't find where the install space is recommended, so asked.

So if you do not specify any rules, your code is not affected at all.
If you specify rules, they get tested.

When the open PR #150 gets merged, then the above won't hold any more right?

As soon as projects start growing you have to take care of dependencies and package relations.

Well managing dependency doesn't require using install space does it?

If I sound as if I changed my poistion on this subject recently, I'm sorry. I might have done that a bit. I noticed adding instructions for install in CMakeLists.txt isn't that too difficult for me but I'm not sure that's the case for all ROS users.

mathias-luedtke · 2017-03-26T17:09:43Z

Perhaps the issue title is misleading.
We don't build directly to install space. We build to build/devel space and install the results to the install space (according to the rules).
The current behaviour is really odd:

build to build/devel space
run build test
clean
build to build/devel space and install to install space
run rostest on install space

So in step 4 we are rebuilding everything from step 1, which doubles the time the build takes.

Hm, I can't find where the install space is recommended

I don't use it locally. But as soon as others depend on my packages, I have make sure that the install space is set-up properly.
So I don't recommend to use (=source) your local install space, but to add install rules for other users.

On our robots (and pool PCs) all users chain to a common (robot-specific) install space that is chained to the global /opt/ros install space.

Well managing dependency doesn't require using install space does it?

No, not directly. But the install rules make the build results explicit.
This way you can identify your build, exec and export dependencies more easily.

130s · 2017-04-07T19:52:26Z

I posted my response #150 (review), and that PR resolves this ticket I think.

Btw, now I'm convinced that recommending install space, even for novice users, is good, after the discussing in buildsystem SIG.

130s added the enhancement label May 27, 2016

130s mentioned this issue May 27, 2016

reduce verbosity #48

Closed

130s added a commit to 130s/industrial_ci that referenced this issue Jun 4, 2016

Use install space by default (addresses ros-industrial#54).

ba6a1bb

130s mentioned this issue Jun 4, 2016

[doc] Use install space by default #60

Closed

130s added a commit to 130s/industrial_ci that referenced this issue Jun 5, 2016

Use install space by default (addresses ros-industrial#54).

6fd2e59

130s added a commit to 130s/industrial_ci that referenced this issue Jun 22, 2016

Use install space by default (addresses ros-industrial#54).

3567ace

130s added a commit to 130s/industrial_ci that referenced this issue Jun 23, 2016

Use install space by default (addresses ros-industrial#54).

fc22604

Omit entire catkin build section when devel space is not choosed to be used.

130s added a commit to 130s/industrial_ci that referenced this issue Jun 23, 2016

Use install space by default (addresses ros-industrial#54).

a3b7324

Omit entire catkin build section when devel space is not choosed to be used.

130s added a commit to 130s/industrial_ci that referenced this issue Aug 30, 2016

Use install space by default (addresses ros-industrial#54).

f77a2ce

130s closed this as completed Sep 12, 2016

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 6, 2017

build to install space directly (ros-industrial#54)

b1e8927

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 6, 2017

build to install space directly (ros-industrial#54)

e515a5b

mathias-luedtke mentioned this issue Mar 6, 2017

[WiP] Fix upstream downstream package logic #137

Closed

5 tasks

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 7, 2017

build to install space directly (ros-industrial#54)

0061446

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 8, 2017

build to install space directly (ros-industrial#54)

670a5db

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 8, 2017

build to install space directly (ros-industrial#54)

3570837

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 8, 2017

build to install space directly (ros-industrial#54)

b06a5ad

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 9, 2017

build to install space directly (ros-industrial#54)

392ebf2

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 11, 2017

build to install space directly (ros-industrial#54)

b5ce2ea

mathias-luedtke added a commit to mathias-luedtke/industrial_ci that referenced this issue Mar 20, 2017

build to install space directly (ros-industrial#54)

7fcafca

130s reopened this Mar 26, 2017

130s closed this as completed Apr 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider building to install space directly #54

Consider building to install space directly #54

130s commented May 27, 2016

gavanderhoorn commented May 27, 2016

130s commented Jun 4, 2016

gavanderhoorn commented Jun 4, 2016

rhaschke commented Jun 4, 2016 •

edited by 130s

davetcoleman commented Jul 7, 2016

130s commented Sep 12, 2016

130s commented Mar 22, 2017

130s commented Mar 26, 2017

mathias-luedtke commented Mar 26, 2017

130s commented Mar 26, 2017

mathias-luedtke commented Mar 26, 2017 •

edited

130s commented Apr 7, 2017

Consider building to install space directly #54

Consider building to install space directly #54

Comments

130s commented May 27, 2016

gavanderhoorn commented May 27, 2016

130s commented Jun 4, 2016

gavanderhoorn commented Jun 4, 2016

rhaschke commented Jun 4, 2016 • edited by 130s

davetcoleman commented Jul 7, 2016

130s commented Sep 12, 2016

130s commented Mar 22, 2017

130s commented Mar 26, 2017

mathias-luedtke commented Mar 26, 2017

130s commented Mar 26, 2017

mathias-luedtke commented Mar 26, 2017 • edited

130s commented Apr 7, 2017

rhaschke commented Jun 4, 2016 •

edited by 130s

mathias-luedtke commented Mar 26, 2017 •

edited