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

embedded docopts.sh wrapper into docopts binary #60

Open
Sylvain303 opened this issue Apr 22, 2022 · 5 comments
Open

embedded docopts.sh wrapper into docopts binary #60

Sylvain303 opened this issue Apr 22, 2022 · 5 comments

Comments

@Sylvain303
Copy link
Collaborator

Sylvain303 commented Apr 22, 2022

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

  • both docopts and shell helper at the same place
  • coherent with example
  • could be also outputted as evaled content
  • could be stored in the $PATH too at user preference ex: docopts get-wrapper docopts.sh > $HOME/bin/docopts.sh

Cons

  • huge impact on eval if the code is also evaled at run time

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

golang/go#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
Copy link
Contributor

agilgur5 commented Apr 22, 2022

  • 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
Copy link
Collaborator Author

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
Copy link
Contributor

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 golang/go#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: moby/moby#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
Copy link
Collaborator Author

@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?

  • add new docopts command argument get-wrapper
  • add the related code (probably as short Go sample above)
  • update README with discouraged eval and reason why
  • add get_docopts.sh automated changes? (was not handled because the repository was downloaded already as a requirement
  • make install?

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 😄

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 🤮 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. 😞

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? 😉

Oh, there's the strange generator for the README with a bash parser here 🤔 🤨

# 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
Copy link
Contributor

agilgur5 commented Sep 10, 2022

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 #59 (comment)

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.

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

No branches or pull requests

2 participants