-
Notifications
You must be signed in to change notification settings - Fork 190
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
Drop short symlink to binary #175
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense to me, and thank you for such a well-justified PR. I agree this is the way forward, however I have some concerns about current users of innernet being confused by a command essentially disappearing from underneath them (assuming that not all users read the GitHub release notes :). Unfortunately since I already let the cat out of the bag with this lazy symlink packaging method - I'm curious if there are any recommendations now for a smooth "transition plan" into using shell aliases vs. symlinks. I'm happy to make the diff larger if there's some temporary code to help migrate people. Ideas: simplest versionKeep the symlink for one more minor version, but add a warning to the command output when more complicated versionProvide the above warning, but also add a feature to "auto-install" the alias for a couple common shells. Not sure if it's possible to add default aliases for shells in typical OS packages similar to completions or not, I haven't looked into it enough yet, but it seems like at the least it might be considered a hack. |
As a Linux packager I can say that bundling aliases out of the box is pretty much a non-starter. Some systems have hacky ways of making this happen, some distros specifically outlaw it. I would suggest a perhaps more aggressive variant of your first suggestion.
If you are less aggressive and just warn people with scripted usage will just ignore it anyway so you are really just putting off their pain for another release with no real gain. If you did want to be softer you could do it your way first, then in the next cycle you could do it my suggested way. Also remember none of this will actually make it to everybody because in many cases distro packages already do their own thing. Dropping the symlink is something you can do for some distros, but some distros (e.g. Arch Linux) will need to drop the symlink in their downstream packaging. Others don't even have it yet so it is all moot. |
@alerque I like the stub binary idea that returns an error code! Since I was planning on doing a release imminently, maybe I'll the "soft" version where in this release I'll cherry-pick 4862984 right now, add code to print a soft warning, and then in the following minor release will do your stub idea. Thanks again for your input from the perspective of a package maintainer, it's really helpful to understand that side of things. |
Agreed. It's been long enough, lol. Sorry @alerque for not having a more deliberate transition plan in place. Perhaps this is a good time to do a minor version bump as well? |
Oh yeah! |
Follow-up to just-merged #175. We no longer install the `inn` symlink, so users doing that manually should know what they are doing.
Follow-up to just-merged #175. We no longer install the `inn` symlink, so users doing that manually should know what they are doing.
I am a relative newcomer to this project, but just deployed it across a couple dozen devices on three continents. Remarkably I only ran into a handful of issues, and for most of those I see open issues for either fixing or documenting them. The most frustrating part of getting started was the confusing perceived lack of documentation. It turns out a lot of that was just a missmatch between the actual code and installations and the readme. The readme and some installations were referencing the client as
inn
, but others only hadinnernet
. It took me quite a while to figure out that the short one was just a symlink to the full binary name and that I could safely ignore it.As a long time sys admin and Linux packager (current Arch Linux TU, once PLD Linux core team member, etc.) I would like to suggest that this arrangement is trouble and should be dropped.
It is difficult to package consistently. Especially for a Rust project without a real application build system (cargo handles the binary build fine but is not equivalent of autotools or cmake or meson or similar for full installs). Some systems (as I ran into) will not end up with the symlink at all. Even
cargo install
directly from crates.io is an example of an install that won't support that properly.It just makes a mess of documentation. Man pages are not accessible as
man inn
, andinn --help
identifies as something other than the $0 as entered.Getting shell completion routines setup is just that much extra mess, and again not all distros are going to even end up with the symlink variant working right.
Some systems won't support symlinks at all. Windows is going to require different shenanigans, Android is going to pose it's own problems with symlinks, etc. Just save people the trouble starting now and use the full
innernet
and only expect that one binarry.There is nothing wrong with short alliases, but for Unix users these should be setup by the end user as
alias inn=innernet
. This will automatically take care of completions for most shells and has lots of other advantages, notably for documentation across platforms being consistent but people that type it a lot can still get a short command name.