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

gazebo: remove #88466

Merged
merged 4 commits into from Jun 4, 2020
Merged

gazebo: remove #88466

merged 4 commits into from Jun 4, 2020

Conversation

puzzlewolf
Copy link
Contributor

Motivation for this change

The packages gazebo{6,7}(-headless) have had a broken dependency sdformat since 2016 66b6c2c. They were not marked as broken themselves, but of course they don't build either.
Searching for "gazebo" in this repo results in many unrelated issues where they annoyingly turn up as a dependent packages which failed to build.

There are no dependencies for these packages, I think it's safe to remove them.
@pxc

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@puzzlewolf
Copy link
Contributor Author

@therealpxc, @acowley, @breakds, @akru I saw your comments in #11713. What's your opinion on this? #11713 sounds like you have moved your ROS packages elsewhere.

@therealpxc
Copy link
Contributor

I haven't used Gazebo in a long time. Removing these and pulling me off the maintainers list seems fair to me.

The others used them more recently, so their input is probably more important.

@acowley
Copy link
Contributor

acowley commented May 26, 2020

Being broken for that long is a pretty good indicator that it should be removed. Hopefully someone looking to get it working again can find the git history to have a starting place. Thank you for taking action on this!

@akru
Copy link
Contributor

akru commented May 26, 2020

Ok, agree.

@puzzlewolf
Copy link
Contributor Author

Thank you all for your answers!

@therealpxc: Ok, I'll remove pxc from maintainer-list.nix, and from the packages where you are listed as maintainer.
I noticed that ignition.transport is broken since 2018, so I'll remove that package too.
Do you think it makes sense to keep genromfs, ignition-math and qgroundcontrol around?

@puzzlewolf
Copy link
Contributor Author

puzzlewolf commented May 30, 2020

To summarize: sdformat, gazebo and ignition-transport have been broken (either silently or marked) since 2016. This PR removes them and ignition-math, which is only used in gazebo. Additionally, their maintainer @therealpxc has asked to be removed, which orphans genromfs and qgroundcontrol.

@Lassulus Lassulus merged commit a152626 into NixOS:master Jun 4, 2020
@puzzlewolf puzzlewolf deleted the gazebo_remove branch June 4, 2020 12:24
@lopsided98
Copy link
Contributor

I have working versions of Gazebo in my nixpkgs fork, which I maintain as part of my project for ROS support in Nix.

I have begun to upstream some of the dependencies (see #85983), and will hopefully be able to create PRs for the rest over the next few weeks.

I'm also willing to take over maintenance of QGroundControl as I use it regularly.

@breakds
Copy link
Contributor

breakds commented Jun 21, 2020

Sorry I wasn't paying enough attentions to the github notifications and late to the thread. Thank you for taking care of the broken package. Also I am really grateful about @lopsided98 's work, which should work for people who want to use Gazebo or other ROS packages in Nix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants