search

docopt / docopts

Shell interpreter for docopt, the command-line interface description language.
Other
504 stars 53 forks source link

embedded docopts.sh wrapper into docopts binary #60

Open Sylvain303 opened 2 years ago

Sylvain303 commented 2 years ago

Following #59 (Search "Sidebar -- docopts.sh entrypoint wrapper" heading)

We discussed how to embed docopts.sh wrapper into docopts binary itself. One of the purpose is to keep a single standalone binary. May be it could also open some new feature and or behavior. So lets explore.

Pros

Cons

Go sample code

package main

import (
  _ "embed"
  "fmt"
)

//go:embed docopts.sh                                                                                                                                
var docopts_sh string

func main() {
  fmt.Println(docopts_sh)
}

Go code ref

https://pkg.go.dev/embed https://go.dev/src/fmt/print.go

https://github.com/golang/go/issues/28822 - discussion about automatic handling NEWLINE

https://en.wikipedia.org/wiki/Newline#Representation (seems macos moved to \n)

from issue above: notepad seems windows 10 to support \n May 8th, 2018 - https://devblogs.microsoft.com/commandline/extended-eol-in-notepad/

agilgur5 commented 2 years ago
  • huge impact on eval if the code is also evaled at run time

Since there's a way around this -- just output to file and you get the same result as right now -- I think we can mostly overlook this con. Just needs to be addressed in the docs in a way that encourages the preferred behavior.

//go:embed docopts.sh                                                                                                                                
var docopts_sh string

I was wondering how to load it at compile-time! Cool to know you can do that with embed!

Looks good to me at least. The only thing I remember having to deal with when outputting like this is line endings between different OS's, but I'm guessing Go handles that properly when cross-compiled (I had to manually change line endings in Bash on different OS's before... do not recommend lol). There are some additional things I've done for completions before (e.g. output helper code to .bashrc), but in this case I think the code you've written is literally all that needs to be done 💯

Sylvain303 commented 2 years ago

handling line endings for different OS

Oh, so long time ago I didn't think about this. That's true. #54 will help to watch for that.

but I'm guessing Go handles that properly when cross-compiled

Do you have proof?

Did you test the code above on mac? If you build it on a macos, some behavior may be handled automatically what about cross-compilation?

under Linux I go 0a LF as expected

./build/docopts_linux_amd64 get-wrapper docopts.sh | xxd
[...]
00001ad0: 7273 6520 2224 7b42 4153 485f 534f 5552  rse "${BASH_SOUR
00001ae0: 4345 5b31 5d7d 2220 2224 4022 2922 0a20  CE[1]}" "$@")". 
00001af0: 2020 2066 690a 6669 0a                      fi.fi.

attached base64 encode cross-compiled binary

from the branch dev-embedded-docopts_sh

$ base64 < ./build/docopts_darwin_amd64 > docopts_darwin_amd64.txt 
$ md5sum ./build/docopts_darwin_amd64
167295ef8fa7d5b6541c939e118c47fc  ./build/docopts_darwin_amd64

docopts_darwin_amd64.txt

where line ending breaks on macos?

Worst case is mixed line ending output. Actually docopts extensively uses \n everywhere in its text processing.

docopts.sh on Linux machine if git doesn't transform NEWLINE will end all its line with \n so I suspect the embedded string is \n terminated when built on such system.

I never had issue yet about line ending on macos.

agilgur5 commented 2 years ago

Hi @Sylvain303, I'm back after a long delay (see this comment for more details on the delay) and did some testing for this!

but I'm guessing Go handles that properly when cross-compiled

Do you have proof?

Did you test the code above on mac? If you build it on a macos, some behavior may be handled automatically what about cross-compilation?

So that was just a guess / assumption; I hadn't tested at the time. I see you edited your opening comment and added a reference to https://github.com/golang/go/issues/28822, which I think answers the question

attached base64 encode cross-compiled binary

So I tested this and it worked fine on macOS, so can confirm now! Thanks for cross-compiling it 👍

where line ending breaks on macos?

Sorry, this was my bad, I looked back at my past code / issues / PRs etc and remembered that this was specifically an issue with Docker: https://github.com/moby/moby/issues/8513 . I was running in Docker and outputted helpers to the native OS, causing some line ending confusion, but that's a Docker-specific issue and, as such, not relevant in this case. Sorry for the false alarm there!

As such I think this code is good to go and would love to see it released so that the wrapper can be embedded into a single docopts binary! 🙂

Sylvain303 commented 2 years ago

@agilgur5

So as the discussion seems to actually brings no blocker, and as the examples also don't use any eval on docopts.sh yet, I think we can go for it.

Should we close this discussion issue and open a working issue with some steps?

docopts command argument for extracting docopts.sh

I tend to move out from --long-option for action and would prefer command type for action. Is this action name get-wrapper OK? I think yes, very explicit :smile: 

Usage: doctops get-wrapper [--may-be-some-newline-hack]

README with discouraged eval and the reason why

The main reason come from my background: I'm a sysadmin who runs bash script with root privileges.

I also tend to strongly disagree with the spread of curl installer.sh | sudo bash :vomiting_face: that has poped on many opensource project. And people blindly copy pasting such command.

This practice obviously save time, but breaks opensource old age philosophy of distributing safe and tested thing. The genius code done in the installer.sh was already done in OS packaging code. And it was done better. Ah, I also hate duplication of code Tip #15 DRY—Don't Repeat Yourself

But the learning curve to become a package maintainer is difficult to go through. :disappointed:

So I mitigated the fact by the get_docopts.sh the good way would be #22

All the above don't need to be said in the README but that's the idea I would like to teach and to work on... Can I clone myself? :wink:

Oh, there's the strange generator for the README with a bash parser here :thinking: :raised_eyebrow: 

# README.md is composed with external source too
# Markdown hidden markup are used to insert some text form the dependancies
README.md: examples/legacy_bash/rock_hello_world.sh examples/legacy_bash/rock_hello_world_with_grep.sh docopts build_doc.sh
  ./build_doc.sh README.md > README.tmp
  mv README.tmp README.md

hum it should still work unchanged.

agilgur5 commented 2 years ago

next steps

Should we close this discussion issue and open a working issue with some steps?

I think we can move straight to PR from here and work out any kinks in code review, if necessary.

Tbh, since you had already made the commit, I thought you were just going to push that. If not, then I can work on a PR with some additional docs etc.

I tend to move out from --long-option for action and would prefer command type for action. Is this action name get-wrapper OK?

Yea I think command is standard for these types of things, like kubectl completion bash as I mentioned in https://github.com/docopt/docopts/issues/59#issuecomment-1105410788

Two words (get-wrapper) might not be as intuitive, what about just wrapper as the command? Not a big deal either way though, IMO.

discouraged eval

All the above don't need to be said in the README but that's the idea I would like to teach and to work on

Yea, you've stated that before. I think a one-liner explanation in the existing README would suffice for this.