Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

[Windows] Use console_scripts to generate portable wstool. #128

Closed
wants to merge 2 commits into from

Conversation

seanyen
Copy link

@seanyen seanyen commented Jan 31, 2019

Use console_scripts to generate portable wstool.

This is verified by https://aka.ms/ros project.

@coveralls
Copy link

coveralls commented Jan 31, 2019

Coverage Status

Coverage decreased (-0.3%) to 81.723% when pulling 6b06c4f on ms-iot:wstool_windows_fix into 354e8fe on vcstools:master.

Copy link
Contributor

@rsinnet rsinnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really cool, is this a new feature?

sys.exit(wstool_main(sys.argv))
except ImportError as exc:
sys.exit("ERROR: Cannot find required rosinstall library version, \
check your installation (also of vcstools) is up-to-date. One frequent cause \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this spacing is going to look really weird on the print-out, right?

Also, more pythonic ways of writing multiline strings are:

    sys.exit("Part of a string "
             "and the rest of it.")

Or

    sys.exit("""\
Part of a string \
and the rest of it.""")

Note the spacing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -110,7 +110,9 @@ def run(self):
cmdclass=cmdclass,
# rosinstall dependency to be kept in order not to break ros hydro install instructions
install_requires=install_requires,
scripts=["scripts/wstool"],
entry_points = {
'console_scripts': ['wstool=wstool.wstool_cli:main']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, is this a new feature? 💃

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This console_scripts seems to be a feature for a while (https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html), but since wstool mostly was used in the system supporting shebang so it probably doesn't matter to generate the console program stub.

However on Windows, it doesn't magically recognize it is a python script so people usually need to do "python wstool" to run it. With this console_scripts help, now on Windows you can also do "wstool". :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@tkruse
Copy link
Contributor

tkruse commented Feb 2, 2019

For consistencies sake, similar changes could be made to rosws and rosinstall.

@seanyen seanyen changed the title Use console_scripts to generate portable wstool. [Windows] Use console_scripts to generate portable wstool. Feb 8, 2019
@seanyen
Copy link
Author

seanyen commented Mar 29, 2019

@dirk-thomas @wjwwood ping. Is it something you can help review (or get it merged)? 😃

@dirk-thomas
Copy link
Contributor

@seanyen I am not a maintainer of this package / repo. If you want to move forward without these changes being merged and released you could also use vcstool instead. It does accept the same input file format as this tool and can be used as a drop-in replacement. In ROS 2 we only use vcstool - for ROS 1 we just never went through and spend the effort of updating existing documentation.

@wjwwood
Copy link
Contributor

wjwwood commented Apr 1, 2019

@seanyen I don't have time to help right now.

I took care of these repositories for many years when no one else would, but now I no longer use these tools regularly and do not have time to continue to help with it. Hopefully someone who uses the tools and has some free time will step up to help maintain.

If you want to move forward without these changes being merged and released you could also use vcstool instead.

That doesn't really help @seanyen get this pull request merged, and without updating all the ROS 1 installation instructions is probably not a very attractive solution.

It does accept the same input file format as this tool and can be used as a drop-in replacement.

It's not a drop in replacement, wstool has several features that vcstool doesn't have. Most of which are geared towards preventing novice users from messing up their source checkouts and helping them deal with updates to the source workspace.

It might work for a drop-in replacement in the ROS 1 install instructions, but it's not a complete replacement.

In ROS 2 we only use vcstool - for ROS 1 we just never went through and spend the effort of updating existing documentation.

We never did that to avoid disruption in ROS 1 and because we never reached feature parity with wstool, which is similar to why we never made catkin_tools the default rather than catkin_make_isolated, despite the fact that it was superior in the default cases.

@dirk-thomas
Copy link
Contributor

That doesn't really help @seanyen get this pull request merged, and without updating all the ROS 1 installation instructions is probably not a very attractive solution.

While it doesn't help with these patches it will allow to provide a working set of instructions for Windows builds. I would assume that the instructions for Windows will need at least some level of difference to the existing instructions targeting Linux / macOS so I don't think it needs to use the same commands for every step.

@tkruse
Copy link
Contributor

tkruse commented Apr 2, 2019

@wjwwood: what is the designated OSRF process for enabling the community to make releases for tools that OSRF cannot maintain due to other priorities?

Teach a man to fish.

@wjwwood
Copy link
Contributor

wjwwood commented Apr 2, 2019

There's two binary artifacts of a release, pypi (pip) and deb's (hosted on packages.ros.org). The latter is difficult to give access to just anyone. I can give permissions for pypi, and help with pushing the debs (takes minutes each release). What I don't have time for is testing the release candidate, writing changelogs, tagging the release, notifying the community/users of breaking changes if needed, etc...

If anyone wants to do that part of it, then they just need to ask and I'll get them setup. I think it will be in much better hands with someone who uses it regularly and has enough bandwidth care for it.

@tkruse
Copy link
Contributor

tkruse commented Jul 13, 2019

@wjwwood , I am already maintainer in

but not

If you add me I may find time to do releases to there.

@wjwwood
Copy link
Contributor

wjwwood commented Jul 15, 2019

@tkruse I don't see your pypi username (might be a weird thing since pypi has been through a few migrations), can you confirm it to me? I'll add you on all of them.

@dirk-thomas
Copy link
Contributor

Please exclude https://pypi.org/project/rosinstall-generator/ from the list of to-be-added projects. I am actively maintaining that repo and would like to continue doing the releases.

@tkruse
Copy link
Contributor

tkruse commented Jul 15, 2019

Sorry, I meant rosinstall_shellcompletion not rosinstall_generator. I assume it is released from https://github.com/vcstools/rosinstall_tab_completion

Userne is https://pypi.org/user/ThibaultKruse/

@wjwwood
Copy link
Contributor

wjwwood commented Jul 15, 2019

I added you on the ones you asked for originally, but I'm not a maintainer on https://pypi.org/project/rosinstall_shellcompletion/, so @tfoote or @dirk-thomas will have to add you.

@dirk-thomas
Copy link
Contributor

I added both of you - @tkruse @wjwwood - to rosinstall-shellcompletion.

@seanyen
Copy link
Author

seanyen commented Jul 30, 2019

@tkruse Just want to check if anything I can help expedite the Windows related fixes for wstool and vcstools. It would be nice to see there are a release functional on Windows on the PyPi.org.

@tkruse
Copy link
Contributor

tkruse commented Aug 4, 2019

@seanyen : I will make a test release on testpypi once I am ready to release, at the time I shall ping you here again so you can verify the test release

@seanyen
Copy link
Author

seanyen commented Jan 15, 2020

@tkruse Happy new year. Just want to ping to see if we can move this forward.

@tkruse
Copy link
Contributor

tkruse commented Jan 15, 2020 via email

@dirk-thomas
Copy link
Contributor

Closing since the repository is about to be archived: see #154.

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

Successfully merging this pull request may close these issues.

None yet

6 participants