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

Provide a 'clean' verb #37

Closed
mikaelarguedas opened this issue Apr 27, 2018 · 31 comments
Closed

Provide a 'clean' verb #37

mikaelarguedas opened this issue Apr 27, 2018 · 31 comments
Labels
enhancement New feature or request

Comments

@mikaelarguedas
Copy link
Contributor

to delete build install and log folder.

Once the notion of profile is there it could delete the folders corresponding to that profile rather than the default folders

@mikaelarguedas mikaelarguedas added the enhancement New feature or request label Apr 27, 2018
@tfoote
Copy link
Member

tfoote commented Apr 27, 2018

Be careful about overloading terms unintentionally. Clean usually only effects intermediate and output files not the installation path. https://stackoverflow.com/questions/1439950/whats-the-opposite-of-make-install-i-e-how-do-you-uninstall-a-library-in-li That's more like uninstall which might be a companion verb.

@mikaelarguedas
Copy link
Contributor Author

That is a good point.
I used the term clean here inspired how the verb is named in catkin_tools.
In this case the new verb will cover both removing intermediate and output files as well as installed files (+ other by-products like logs of the tool itself that don't really fall in either categories).
I don't think has an equivalent term in make. I'm open to suggestion for any name that can encompass the described behavior.

@dirk-thomas
Copy link
Member

I think we all agree that the functionality described in this ticket is useful and should be added.

The point @tfoote is trying to make is that clean is a target in Makefiles generated by CMake. And its semantic is different: it removes build artifacts but it doesn't remove anythin else like Makefiles, CMake cache, etc. Therefore it might be better to use a different term.

@tfoote
Copy link
Member

tfoote commented Apr 27, 2018

How about clean that just passes through to the default Makefile/CMake generated targets. And an additional verb wipe or expunge which will remove the installed contents too. We could make those shortcuts for a fully named verb. wipe-install or expunge-install or if it's in long form erase-install could also work. And these more powerful verbs will chain through and call clean too.

@dirk-thomas
Copy link
Member

dirk-thomas commented Apr 27, 2018

How about clean that just passes through to the default Makefile/CMake generated targets.

That is already possible with colcon build --cmake-target clean. So I don't see a need to elevate it into a verb.

@Theosakamg
Copy link

That is already possible with colcon build --cmake-target clean. So I don't see a need to elevate it into a verb.

@dirk-thomas in case of other sub-system than cmake ? (eg. Gradle...)
It possible to apply the same model with colcon build --gradle-task clean.
But in case of multi-packages (mix of cmake and gradle) you need to pass colcon build --cmake-target clean --gradle-task clean ?

And in case of gradle (seem to be same for cmake), target clean only rm -rf the build folder and not make a build after. it was disruptif with verb build.

(just a point of view of novice user.)

@dirk-thomas
Copy link
Member

I am not familiar with Gradle and don't know what it exactly does when running the clean "target". In the case of CMake / Make the clean target only removes the object files / generated files. It does not remove the build directory, e.g. the CMakeCache.txt file remains. It also doesn't affect the install space.

For the clean verb described in this ticket the scope would be to allow removing the actual build and install directories of a package (basically "rolling back" the build step which was run before).

For the case of invoking the existing clean target of some build systems I think the "mixin" approach described in #94 (comment) would be more suited.

@dirk-thomas
Copy link
Member

For the case of invoking the existing clean target of some build systems I think the "mixin" approach described in #94 (comment) would be more suited.

The mixin concept is now available through colcon-mixin.

@scpeters
Copy link

I hear @tfoote's point that catkin clean is different than make clean, but it would be easier for me as a user if it worked the same way as catkin-tools. For me, it's more confusing for colcon clean to be different from catkin clean than if it's different from make clean.

@koenlek
Copy link

koenlek commented Dec 19, 2018

I agree with @scpeters, I'd prefer clean to work like it did in catkin tools.

An important point of running clean is that it clears caches. As far as I know, install space will also have the problem that old files won't be removed. Let's say I used to have a node bar in my foo_pkg, but that I renamed bar to bar2, then after subsequent builds, the install space would have both a bar and bar2 node.

@tprk77
Copy link

tprk77 commented Mar 2, 2019

Also agree with @scpeters. An analogous command to catkin clean -y would be ideal, in my opinion.

@ruffsl
Copy link
Member

ruffsl commented Aug 26, 2019

Over on #213 (comment) , I have described a use case for this kind of verb. I'd like to request for the ability to selectively control the clearing of the build vs install sub folders on a per package level for optimizing build caching for incremental builds. Comment on there if you have further suggestions.

I'm working on the assumption that the workspace was built using isolated install, and the source may have been changed between builds, so using a classic uninstall function from the build system may not completely clear the install folder. Having a verb to selectively delete these folder would spare the user from having to infer the base path for all build and install directories and enumerate purging of packages.

BTW, what would this verb mean for logs? If a package was specified, would the verb clear the package sub directory in all time-stamped directories in the log folder?

What should this do if a non isolated install is used, but a package name is given? Just error out?
Could catkin tools clean on a per package, per build-vs-install basis? I'm guessing not with merge install.

@dirk-thomas
Copy link
Member

If a package was specified, would the verb clear the package sub directory in all time-stamped directories in the log folder?

Without an explicit option for cleaning logs I don't think this verb should touch the log directory.

Though an explicit option like "clean-logs --all / --older-than-a-week" could certainly help deleting obsolete log directory. But this functionality shouldn't be intersecting with the package selection arguments. I don't see a reason to only the logs of a specific package.

What should this do if a non isolated install is used, but a package name is given? Just error out?

This is where you want / need the uninstall rules in order to be able to achieve the requested behavior.

@ruffsl
Copy link
Member

ruffsl commented Aug 26, 2019

Without an explicit option for cleaning logs I don't think this verb should touch the log directory.
I don't see a reason to only the logs of a specific package.

That's what I was feeling, as logs have lifetimes beyond that artifacts in build/install directories.
Given how they are organized, there's not much cause to clear logs by package rather than time.

@mikepurvis
Copy link
Contributor

mikepurvis commented Oct 30, 2019

Note that catkin clean although convenient has some goofiness going on behind the scenes. Basically, it's two totally different behaviours depending what you invoke:

  • catkin clean deletes the build, logs, devel, and install folders, if they exist.
  • catkin clean mypackage deletes build/mypackage and if symlink devel is enabled, removes the generated files corresponding to the particular listed package(s). I don't believe it touches the logs folder.

Not to say that these aren't both pretty convenient, but for implementation and user sanity, I would support them having different names.

@scpeters
Copy link

scpeters commented Nov 6, 2019

Note that catkin clean although convenient has some goofiness going on behind the scenes. Basically, it's two totally different behaviours depending what you invoke:

Perhaps separate verbs?

  • catkin clean-workspace
  • catkin clean-package

@danthony06
Copy link

Is anyone interested in reviving this work? After doing some ROS 2 development, I'd really appreciate this functionality.

@ruffsl
Copy link
Member

ruffsl commented May 22, 2021

Perhaps separate verbs?

  • catkin clean-workspace
  • catkin clean-package

Hey folks! I made a quick implementation by coding this into two subverbs, allowing users to clean base folders within a workspace or on a per package bases. You can check it out and try it yourself here:

colcon/colcon-clean#4

colcon clean --help
colcon clean workspace --help
colcon clean packages --help
colcon clean packages --packages-above colcon-cmake
Paths:
     /home/ruffsl/ws/colcon/build/colcon-cmake
     /home/ruffsl/ws/colcon/build/colcon-ros
     /home/ruffsl/ws/colcon/install/colcon-cmake
     /home/ruffsl/ws/colcon/install/colcon-ros
Clean the above paths? [Y/n]

colcon clean packages --packages-above colcon-cmake -y

Perhaps we could upstream this into the colcon org, and bundle it up with colcon-common after some iterations and PRs?

@ruffsl
Copy link
Member

ruffsl commented Jun 9, 2021

For potential uses of colcon clean, here is an example where the packages subverb is used to clean the respective install and test result bases for stale packages and keep CI cache sanitary:

ros-navigation/navigation2#2399

@AndyZe
Copy link

AndyZe commented Sep 20, 2021

This reminds me of Bill Gates admitting Ctrl + Alt + Delete was a mistake. Let's admit it: colcon clean would be really useful and we should probably overlook the quibbles here and make it happen

@ruffsl
Copy link
Member

ruffsl commented Sep 20, 2021

Has anyone had the chance to try out the colcon clean extension I posted about above?
I have since added a full readme with documentation on it's current functionality:
https://github.com/ruffsl/colcon-clean

Perhaps we could migrate the extension repo to this colcon org and release it upstream. I think the scantree dependency for fast file path globbing may be the only issue, but perhaps that could be swapped an alternate regex approach.

@davetcoleman
Copy link

I miss catkin clean and agree this would be beneficial!

@scpeters
Copy link

@cottsay what is the procedure for adding https://github.com/ruffsl/colcon-clean to the colcon org?

@tfoote
Copy link
Member

tfoote commented Jan 4, 2022

It looks like @ruffsl's implementation with specific subcommands resolves my worries about clarity of the command.

@ooeygui
Copy link

ooeygui commented Jan 7, 2022

I started going down the path of adding ROS: Clean to the Visual Studio Code ROS Extension per ms-iot/vscode-ros#554. Would love to see @ruffsl's implementation adopted in colcon instead of brute forcing it from the extension.

@kledom
Copy link

kledom commented Jul 27, 2022

@cottsay Friendly ping!
Any plans to make colcon-clean installable via apt?

@cottsay
Copy link
Member

cottsay commented Jul 27, 2022

@ruffsl - if you'd like to proceed with moving colcon-clean under the colcon org and releasing debs for it, I can help make that happen. We should open a new issue to specifically track it. #493 is an example of a migration we completed previously.

@ruffsl
Copy link
Member

ruffsl commented Jul 28, 2022

@cottsay - I have opened two new tickets requesting the transfer and release of the new relevant extensions:

@ruffsl
Copy link
Member

ruffsl commented Dec 15, 2022

Hey folks, colcon-clean has been released, so I think we can finally close this ticket! #521 (comment)

$ sudo apt install python3-colcon-clean 
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
  python3-pathspec python3-scantree
The following NEW packages will be installed:
  python3-colcon-clean python3-pathspec python3-scantree
0 upgraded, 3 newly installed, 0 to remove and 16 not upgraded.
Need to get 51.5 kB of archives.
After this operation, 258 kB of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 http://us.archive.ubuntu.com/ubuntu jammy/universe amd64 python3-pathspec all 0.9.0-1 [28.8 kB]
Get:2 http://packages.ros.org/ros2/ubuntu jammy/main amd64 python3-colcon-clean all 0.2.0-1 [9,898 B]
Get:3 http://us.archive.ubuntu.com/ubuntu jammy/universe amd64 python3-scantree all 0.0.1-2 [12.7 kB]
Fetched 51.5 kB in 1s (39.1 kB/s)        
Selecting previously unselected package python3-pathspec.
(Reading database ... 317982 files and directories currently installed.)
Preparing to unpack .../python3-pathspec_0.9.0-1_all.deb ...
Unpacking python3-pathspec (0.9.0-1) ...
Selecting previously unselected package python3-scantree.
Preparing to unpack .../python3-scantree_0.0.1-2_all.deb ...
Unpacking python3-scantree (0.0.1-2) ...
Selecting previously unselected package python3-colcon-clean.
Preparing to unpack .../python3-colcon-clean_0.2.0-1_all.deb ...
Unpacking python3-colcon-clean (0.2.0-1) ...
Setting up python3-pathspec (0.9.0-1) ...
Setting up python3-scantree (0.0.1-2) ...
Setting up python3-colcon-clean (0.2.0-1) ...

$ colcon clean --help
usage: colcon clean [-h] {packages,workspace} ...

Cleans package workspaces.

options:
  -h, --help            show this help message and exit

colcon clean verbs:
  packages              Clean packages in workspace
  workspace             Clean current workspace

  {packages,workspace}  call `colcon clean VERB -h` for specific help

@cottsay cottsay closed this as completed Dec 15, 2022
@phil123456
Copy link

E: Unable to locate package python3-colcon-clean
Ubuntu 20.04

@ruffsl
Copy link
Member

ruffsl commented Sep 10, 2023

E: Unable to locate package python3-colcon-clean
Ubuntu 20.04

Due to missing python dependencies available with and after jammy, packing this extension couldn't be backported to Ubuntu focal.

#521

You could try still try using pip though.

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