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

catkin could advertise the buildspace #304

Closed
tkruse opened this issue Jan 13, 2013 · 39 comments
Closed

catkin could advertise the buildspace #304

tkruse opened this issue Jan 13, 2013 · 39 comments

Comments

@tkruse
Copy link
Member

tkruse commented Jan 13, 2013

Based on this discussion: #294

Currently sourcing devel/setup.bash creates an env from which the develspace and source spaces can be derived. However, the buildspace cannot be derived from the environment.

This ticket suggests that catkin defines a way to create an environment that includes a pointed to the build space. This can be used for global build command invocation or quick navigation to the build space.

One possibility would be top have a file generated like:

workspace/build/setup.*sh

which sources the devel space setup.*sh, but also sets some Environment variable.

In the case of isolated build spaces, this setup is not too useful, but catkin_make_isolated could be extended to create a parent build-space for all isolated build spaces, into which such setup.*sh files would fit into.

Another alternative would be for catkin_make to generate setup.*sh files in the workspace root. However that still hides the detail of the build sace which can be useful for convenience tools.

@dirk-thomas
Copy link
Member

Please describe a specific use case which can currently not be realized but would be implemented. Also please describe how that should work in detail.

According to the description above the user would require to source the different setup from build space which seems to be not a good idea.

@tkruse
Copy link
Member Author

tkruse commented Jan 29, 2013

The use case is for the user to invoke build from anywhere on the filesystem easily without having to navigate to a specific folder via an absolute or relative path.

So at least one of these should be implemetable after this issue is fixed:

roscd; catkin_make
roscd; make
roscd foopkg; catkin_make

Currently no tool can implement this behavior based on catkin information alone, because sourcing the devel/setup.sh does not give a hint to the buildspace(s).

@dirk-thomas
Copy link
Member

Case 1 and 3 are not valid since catkin_make does not allow the invocation in the build folder.
Case 2 is not catkin related at all. So I don't think that catkin should address case 2.

@tkruse
Copy link
Member Author

tkruse commented Jan 29, 2013

What cakin_make allows today is irrelevant, what matters is what
catkin_make could allow in the future. It would have to be changed as well,
obviously, but that's for another ticket. And case2 is only possible if a
catkin offers a convenient way to locate a build space, so it is
catkin-related in my mind. no other too in the workflow can make case2
possible.

On Tue, Jan 29, 2013 at 2:04 AM, Dirk Thomas notifications@github.comwrote:

Case 1 and 3 are not valid since catkin_make does not allow the invocation
in the build folder.
Case 2 is not catkin related at all. So I don't think that catkin should
address case 2.


Reply to this email directly or view it on GitHubhttps://github.com//issues/304#issuecomment-12815016.

@tkruse
Copy link
Member Author

tkruse commented Feb 5, 2013

Based on Discussion, this could be achieved with a marker file. The marker file should store buildspace and source space. When invoking catkin_make without arguments that would create a new buildspace, catkin_make should fist check whether the current path is a subfolder of an existing catkin_workspace. If so, it should read the marker file. If the buildspace declared there exists, it should invoke make (+cmake) there. If the buildspace does not exist, it should create a new one with the sourcespace from the markerfile. If no markerfile was found, catkin_make should proceed as now, trying to create a new workspace if the src folder exists.

@tkruse
Copy link
Member Author

tkruse commented Feb 5, 2013

Sorry, I believe we also need to store the develspace and installspace location in the marker file, as this information also gets lost when we delete the buildspace. Other options, like -DBUILDTYPE should IMO not be stored in the marker file. he idea is that catkinmake without further options acts like cmake without further options.

Similarly, the marker file should only yet always be updated, whenever the source, build, devel or installspace are changed.
A special case is when only the buildspace is being changed, I feel there might be some case to be made that if the user points to a different buildspace that exists, the intention of the user regarding source, devel and installspace is unclear, so maybe that should raise an error. e.g.
user has src, build1, devel1, build2, devel2;
build1, devel1, install1 is in his marker file
User runs catkin_make --build build2
does that mean users wants build2 to make into devel1?

To be discussed, maybe in buildsystem SIG.

@dirk-thomas
Copy link
Member

The devel and install locations must not be save in the marker file. We said that when you remove the build folder you are basically removing the configuration and have to specify all arguments again.

@tkruse
Copy link
Member Author

tkruse commented Feb 7, 2013

devel and install locations discussed further in buildsystem sig, no decision yet.

Something else came up, what if the user runs catkin_make when he is inside a build, devel or install space? building into a different build space might be non-inutitive then, in particular if we want to keep the metaphor of a make wrapper. Ideas?

@tkruse
Copy link
Member Author

tkruse commented Feb 19, 2013

More discussions and ideas here. https://groups.google.com/forum/?fromgroups=#!topic/ros-sig-buildsystem/_1PWeIZvAbw
@wjwwood: you wanted a bump on this.

@NikolausDemmel
Copy link
Contributor

Having read up a bunch on this in old threads just now, it seems a couple fruitful discussions and ideas have been abandoned since, or where would one find the most recent developments on this topic?

@tkruse
Copy link
Member Author

tkruse commented Feb 28, 2014

Hi Niko,

as far as I can remember the last time those things were discussed was here:
https://code.ros.org/lurker/message/20130205.233149.6624b156.en.html

I believe my last post there summarized what was discussed online and in live meetings. Not sure whether anyone still has the meeting notes from those meetings.

This was shortly before Willow Garage announced it's "Change" and afterwards priorities obviously shifted.

So the decisions were never officially changed AFAIK, just never implemented. I am not sure whether PRs regarding this would be accepted, since OSRF has less maintenance resources than Willow Garage.

@NikolausDemmel
Copy link
Contributor

That's a shame. I was looking into ways to bring rosemacs up to speed with catkin & co without extensive hacking and this would be one component to that end (but immensly useful even without considering rosemacs).

@NikolausDemmel
Copy link
Contributor

It seems yujin_tools [1] has quite a large set of convenience tools. Instead of a marker file they use a cmake cache file that lives on workspace top level.

[1] https://github.com/yujinrobot/yujin_tools

@tkruse
Copy link
Member Author

tkruse commented Feb 28, 2014

I don't see at a glance where yujin_tools does that.

An initial goal of catkin was to heavily rely on cmake concepts rather than using ros-specific things like the ROS_PACKAGE_PATH, or a .rosinstall file. This might be a good decision if you do not want to provide other convenience tooling for the build process, such as rosbuild did. Initially there should not even have been a catkin_make script.

Given that [parallel_]catkin_make_isolated relies a little less on cmake for organizing workspace builds, I think it would be difficult to use a cmake cache folder as a marker resource for the workspace.

@NikolausDemmel
Copy link
Contributor

If you checkout the init tools [1], there is yujin_init_build, which generates a config.cmake which some relevant info for running cmake inside a build folder. I'm not saying this can replace the proposed marker file for all use cases, but it can for some, e.g. cleaning by wiping the build folder and the reconfigure with the same set of options (at least for basics like src/devel/build/install folders and underlays etc).

[1] https://github.com/yujinrobot/yujin_tools/wiki/yujin-init

@dirk-thomas
Copy link
Member

The ticket is marked with the milestone "untargeted" because the maintainer (which is me) does not expect to spend any time on this feature. If someone would like to add such a feature and takes the lead on this, figures out the details and solves all related problems I am more than happy to discuss proposed concepts if they are detailed enough and also integrate the changes if they are small (or review contributed pull requests).

The problem with this idea is that until now nobody has come up with a clear enough proposal on how this is supposed to work well enough in all currently possible variations. Just a short (far from being complete) list of scenarios to consider:

  • one workspace can be build multiple times e.g. with Debug and Release with diffferent build/devel/install locations
  • one workspace can be build with a single CMake invocation or in isolation
  • while you don't have to specify configure arguments like this on repeated invocation you still need to specify custom build/devel/install folders

@NikolausDemmel
Copy link
Contributor

@dirk-thomas: I completely understand your position as the maintainer. After reading quite large amount of discussion happening about a year ago, I was just curious where this went since. However @tkruse 's answer with regards to Willow makes sense.

@dirk-thomas
Copy link
Member

@NikolausDemmel The "lack of progress" on this ticket is not related to the changes at Willow. The catkin package has been very actively maintained and developed in the last year when looking at the number of addressed bug and feature request in that time frame.

@tkruse
Copy link
Member Author

tkruse commented Feb 28, 2014

Nobody suggested that there is not enough catkin maintenance, so no need to get defensive.

I know that if willow had not changed, I personally would have continued to actively pursue this issue, instead of letting it be left untargeted.

There was already consensus about a marker file that has no problems, such as there being merely an empty marker file marking the workspace root, allowing to invoke catkin_make from anywhere within. This alone could already have been implemented without further considerations, as a starting point for further discussions.

If the marker file additionally had content, some design decisions arise on what to store and how, but there again a consensus had already been reached. The consensus was:
"the marker file will remember the folder layout from any last call, so removing all build artefacts
and running catkin_make again shall create the same folder layout again."

This was a rather minimal consensus that could be extended later, but a consensus it was.

@dirk-thomas
Copy link
Member

The so called "consensus" has imo several severe drawbacks and not even close to a solid "solution".

I will give just one simple example - consider calling the following commands:

catkin_make -DCMAKE_BUILD_TYPE=Debug
catkin_make -DCMAKE_BUILD_TYPE=Release --build build_release

Afterwards when invoking catkin_make without any arguments it is currently the case that the debug build with its default build folder is invoked again. With your approach the release build would be selected instead. Why is that problematic?

First imo it is rather unintuitive that the same invocation of a command leads to completely different behavior. Second consider what you would need to do in order to build debug again. It would be necessary to explicitly specify the default build folder (which the user has not even used in the initial invocation).

This example as well as many other issues are reasons why it has not just been done yet. If there is a strong interest in these features some people need to spend time on this and come up with a solid concept which considers all of these cases - or a selected subset in a consistent manner.

@tkruse
Copy link
Member Author

tkruse commented Mar 1, 2014

All those cases had been considered, and the result was that even with these drawbacks catkin_make would be extremely more useful to most ROS users (maybe not to you) than in it's current state. Considering these cases again will not change that result.

There are simpler alternatives with less conflict available, such as an empty marker file, or a marker file only specifying the sourcespace (for integration with wstool). With minimal foresight those alternatives can be implemented to make the toolset more useful today and allow future extentions.

If you want people to come up with good solutions, give them something to start with.

@jack-oquin
Copy link
Member

👍

@wjwwood
Copy link
Member

wjwwood commented Mar 1, 2014

If you want people to come up with good solutions, give them something to start with.

From my perspective if you (or anyone else in the community) thinks that the idea is free of conflicts and ready to be acted on, then there is nothing stopping you from acting on it. I just don't see anything blocking a pull request or a prototype in another repository.

As a maintainer of catkin, if an idea presented by the community has been presented in a clear manner and in my opinion is feasible and important, I will usually spend time on it. At the moment I don't think this is a high enough priority to spend time on it, and it is not clear to me that this would be straight forward to implement in a way that wouldn't end up being counter intuitive and confusing (that is why we have marked it as untargeted). And for all of the recent discussion about why no progress has been made, no progress has been made, so I think I have to give all of this recent discussion a 👎.

@NikolausDemmel I apologize for my colleges and community peers, they don't always act professionally or politically. Please do not let this discourage you from contributing to this discussion or prototyping changes to the tools if you think it is feasible and important.

@tkruse
Copy link
Member Author

tkruse commented Mar 2, 2014

@wjwwood : progress has been made on this discussion. I offered the possibility to reduce the issue to:

"catkin should provide a python function to retrieve the location of the respective workspace root given any file path that is below that workspace root, where workspace root refers to the folder in which the catkin_make command is viable"

(workspace root as opposed to the source space, which confusingly still has the catkin_init_workspace command, which btw should be renamed to e.g. catkin_init_sourcespace by now, or something else that does not conflict with the definition of a workspace in REP128)

No part of catkin would need to consume that information, so no new conflicts can arise. This would just be part of the public catkin API that others can use to invoke catkin_make from other places and use a logical folder for maintaining and using further meta-information about workspaces (preferences and configuration history).

A tool like rosemacs can then offer to build the workspace of the currently open file, something that catkin_make so far has made impossible by requiring to know the location of the workspace root to invoke catkin_make, yet obfuscating the location of the workspace root.

As icing catkin could even offer this python function as a shell command for easier consumption by bash scripts. To be lazy catkin_make could just define a marker file/foldername and not offer any python/shell function, making it less future-proof, but also less hassle for you.

Technically it can be achieved by writing a marker file/folder after creating the build folder was successful, but that is up to the catkin maintenance team to decide.

Of course catkin_make could also be extended then to use that information to allow invoking it from anywhere below the workspace root. But I will not ask for so much since I have to fear that nothing at all will happen if I ask for more, based on the past experience.

@NikolausDemmel
Copy link
Contributor

http://youtu.be/FONN-0uoTHI :) -- I can relate to all your opinions why progress hastn't been made as quickly as would have maybe been expected given the wealth of discussion early 2013. I value all contributions to the discussion, raising of concerns and suggestions for solutions. In the end for me it only matters what will happen in the future.

@tkruse Just as a side note (I'm not saying a marker file is bad): You suggest a python API function for better future-proofness, however if this is implemented with help of a marker file, people will use the marker file rather than the python API (especially if their tool is not implemented in python). Having a shell command might help, but a marker file is just too convenient to find to not use it.

@ all: I just want to throw in what people are doing with yujin_make to circumvent the current situation: They assume default names for src/build/devel and only one build folder per workspace. They furthermore have a seperate init step to set up a workspace for the first time and install a config.cmake cache file, which they populate with info from command line options that are more convenient than some of the wieldy -D options or the workspace chaining through repeated sourcing of setup.bash files. It allows them to do things like yujin_init_build --install=/opt/yujin/groovy --underlays="/opt/yujin/groovy;/opt/ros/groovy" . They furhtermore inject a custom environment valiable YUJIN_MAKE_ROOT in the devel/setup.sh file and use it to determine where to find the workspace. This way, with a sourced setup.sh, they can yujin_make from anywhere.

I realize this covers only is a limited use case and I'm also not claiming that this solution is ideal, but it is an example of what /kind/ of things catkin ideally should enable to be done. Over at yujin this solution seems to solve real world problems for them.

Right now, I don't have the time and mainly expertise/experience to drive this myself. It seems while a minimal version of this could be implemented rather easily, as mentioned it requires some amount of foresight. Many things have been considered one way or the other in previous discussions, but I currently woundn't trust myself to make judgement on that.

Because I don't see the whole picture yet, I'm not sure if I'm on the side @tkruse and @jack-oquin, opting to have a minimal solution asap. I feel like the situation is complex enough with many opinions and things to consider to validate a proper REP summing up all the possible solutions and pros and cons, with possibly other tools being affected or considered to be integrated (wstool, rosinstall, ...?). However, for that, someone will need to invest significant time, so we would wait until it is high enough on someones priority list?

@dirk-thomas
Copy link
Member

@tkruse Please provide a pull request against indigo-devel to add the feature as you suggested it. We can discuss the details in that pull request and merge it as soon as possible. If it doesn't change existing API / naming it can also be backported to groovy and hydro.

@NikolausDemmel The yujin tools have the advantage to limit their supported use cases to a subset of the potentially general use cases. Therefore they can provide better support for those specific one. I am not sure how catkin can provide better support for those use cases while maintaining general applicability. E.g. I would consider being able to build different build types in the same workspace an important feature.

@tkruse
Copy link
Member Author

tkruse commented Mar 3, 2014

I will see if and when I have time to make a PR.

@NikolausDemmel : In the consensus of last year, it was agreed that in the default usage catkin_make would also only support single-build-folder workflows.
This is stated in the first sentence of the first post here:
https://groups.google.com/forum/#!topic/ros-sig-buildsystem/_1PWeIZvAbw/discussion
The reason then as well as now is that 99% of ROS users do not even use multiple build configurations, and the few who do are experts enough not to need a maximum intuitive tool, they can supply an additional --build param when they need it, or find other ways to work.

Similarly the wrapping of -D commands had also been suggested for catkin_make by me a year ago, as well as the workspace root env variable, but there was no consensus for that at the time, and the little added value was not worth the effort (Given all the other serious usability issues of catkin), so I was happy enough to get a consensus on a marker file.

Regarding the marker file: Yes, people will use that directly, but that's only a problem in theory, not in practice. It may be nice to amend REP128 to specify the marker file, but that can come later.

@tkruse
Copy link
Member Author

tkruse commented Mar 3, 2014

#603

@stonier
Copy link
Contributor

stonier commented Mar 10, 2014

Our main motivation for the yujin_tools scripts was initially for doing parallel builds (cross-compile and native), but it expanded to include alot of the things getting discussed here so I thought I'd chime in.

When setting out to work on them I was also frustrated with catkin_make and initially wanted to push things into catkin as well. I don't think this is the easiest way to do it anymore though.

catkin_make is just a simple script for compiling provided by the catkin maintainers. It could be a little less perverse in some places, but it's fine for a beginner/simple user. Even if the catkin maintainers had time (@dirk-thomas) he's not the right person for writing a power user's tool for catkin. Simply because he is the provider, not the power user. And I don't think one tool can cover all user's bases here either (e.g. simple vs complex).

Having said that, most of the things discussed here I already do in the yujin_tools.

  • Parallel build spaces with one source workspace : cross-compile, native, debug, release
  • An environment variable pointing to the workspace root : let's you run yujin_make from anywhere once you've sourced setup.bash
  • A customisable workspace rosinstall repository (e.g. yujin_init_workspace ./navigation navigation to install navigation stack sources).
  • An easy rosdep install mechanism in yujin_make (though this should be shifted to yujin_init_workspace.

If it is really the case that we're already implementing things that power users wish to see, I'd be happy to shift the direction to ship it as a more official power user's tool for catkin and slowly incorporate other features.

@tkruse
Copy link
Member Author

tkruse commented Mar 10, 2014

Hi Daniel,
I am not too concerned about power users, and yet I think Dirk is both a Power user, and he intended to write a tool for power users as a main target audience, where power users in his mind want tools that just do one thing each, and have no convenience bloat. Power users have the competence to write their own scripting for their specific needs.
In that respect I believe catkin_make already supports power users as well as possible.

Also I like to quote you from ros-users: "We even go to some of this effort internally and have a few tools that make life easier for interns/engineers who aren't so familiar with ros and are
often only temporarily in and out of the ros team."

To me those interns/engineers are similar to most ros users in academia, and I would not call them power users. But this is the group where IMO ROS should provide tooling to reduce the learning curve and provide early passion for ROS. Luckily this group has no need for for separate build spaces, cross-compiling, or any of the other fancy stuff possible with catkin. So the defaults of catkin_make should in my opinion make it easier to get a simple workflow, while the non-defaults should still allow all the power user features. What is not possible is to have defaults that conveniently support all cases, but that is also unnecessary. In other words, make it easier for the many at the cost of making it harder for the few.

By now I think catkin_make will never reach this ideal while Dirk is maintaining it, because he cannot be convinced of that ideal. So I think the best that can be achieved in the future is to make catkin provide a better mere API for customization and tool integration, so that people can independently of Dirk achieve that ideal based on catkin_make and provide what the ROS community actually needs, without changing the catkin defaults that have been set in stone by now.

Such a catkin API allows to set and retrieve all it's defaults and used build parameters, so that defaults can be changed, invocations wrapped and cached, and bridges to other tools like wstool/rosinstall build in standard ways, without creating schisms in the ROS community.

Such an API can then also support any special interest group that shares a common workflow, such as having a company-standard fixed set of 3 build spaces (maybe with company-standard cmake params) like the ones you mention. With an API, tooling can be provided on top of catkin_make without code duplication and without requiring non-standard assumptions. PR #603 is the first step in that direction.

If yujin_tools could provide better wstool and rosdep integration based on the future catkin API for tool integration, I think that would benefit everybody, both outside and inside yujin. You can help by shaping the API catkin needs to provide to toolsets, such as revealing the locations of source and build spaces, rather than requiring users to remember them and pass them to other tools manually.

@tfoote
Copy link
Member

tfoote commented Mar 10, 2014

Hi guys, please do not use ad hominem arguments they are not appropriate
for this sort of discussion.

@tkruse
Copy link
Member Author

tkruse commented Mar 11, 2014

http://en.wikipedia.org/wiki/Ad_hominem
"An ad hominem (Latin for "to the man" or "to the person"[1]), short for argumentum ad hominem, is a general category of fallacies in which a claim or argument is rejected on the basis of some irrelevant fact about the author of or the person presenting the claim or argument."

I did not reject Dirks stance based on any fact about him as a person, so I did not use an ad hominem argument. I clarified that his ideal is different from mine, and that our views are mutually incompatible.

It is necessary to clarify which person wants which change for catkin, so that the birds of a feather can unite and achieve progress.

This should be much more productive than falsely assuming everybody wants the same, and then always suffer the same conflicts over and over again, with escalation occurring repeatedly as seen in this thread.

@tkruse
Copy link
Member Author

tkruse commented Mar 16, 2014

While we are at it: another theoretical alternative would be to undo this other bad design decision about catkin_make, that setup.sh needlessly sets build environment parameters, like IMO abusing the cmake_prefix_path to allow runtime lookup of packages and chaining workspaces. This was once "justified" by saying that a catkin workspace is merely yet another cmake project (which would not really justify it anyway), but given catkin_make_isolated even this flimsy justification becomes obsolete.

This is also a conceptual hack mixing up runtime and buildtime semantics. If the setup.sh did one thing only, setting up a runtime environment without modifying the build environment needlessly, things would start to make more sense again. Chaining workspaces would then require some other mechanics.

Note also again that the naming of concepts in catkin is a mess, because "chaining workspaces" does not really chain workspaces, but devel/installspaces.

It has all been said before, but since Niko might become a platform tools contributor, I am happy to repeat.

@NikolausDemmel
Copy link
Contributor

Thanks @tkruse for that interesting point I was not conscious about. What exactly do you classify as runtime environment? Everything needed to find packages and run executables? Or is there more to it?

@tkruse
Copy link
Member Author

tkruse commented Mar 19, 2014

Just that rosrun/roslaunch work. With rosbuild both run and build env were prepared by sourcing setup.sh, and it made sense. In catkin, that is much more problematic.

@NikolausDemmel
Copy link
Contributor

OK, this also means splitting up env_hooks into build and run env hooks. I'll keep this on my list of things to possibly look into one day.

dirk-thomas added a commit that referenced this issue May 1, 2014
Marker file for catkin_make (#304)
dirk-thomas pushed a commit that referenced this issue Jun 3, 2014
@dirk-thomas
Copy link
Member

With catkin_tools addressing these use cases I assume this ticket can be closed?

@tkruse
Copy link
Member Author

tkruse commented Mar 20, 2015

Since leaving this ticket open won|t change anything, I guess it can be closed.

@dirk-thomas
Copy link
Member

That was not my actual question, but then I will go ahead and close it.

It can always be reopened if there is need and/or a PR can be proposed.

@dirk-thomas dirk-thomas removed this from the untargeted milestone Aug 6, 2019
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

No branches or pull requests

7 participants