-
Notifications
You must be signed in to change notification settings - Fork 70
Make tools importable #114
Comments
Yes I have. I've started to turn some of them into libraries, but I've been more focused on my decimal library as of late. I plan to spend more time on this library once v3.0 of my decimal package drops, which should be whenever trig functions are added. If you'd like to help in any way you're more than welcome. I'm down to finally make this library useful and help you out! |
Great to hear that! I won't submit a PR right away, as this would require quite a bit of design and refactoring, and I'm not familiar with this codebase. And it would likely save everyone time if you have a look at it first. When you start working on this or have a design/prototype, do let me know and I'll be happy to help - be it reviews, testing, or coding. |
Sounds good. I might fiddle around with it a bit today. If you don't hear from me in a week or so, feel free to ping me. I don't mind being bothered. I'm glad somebody's getting use of this library! |
So, I spent a little while and sketched out an implementation using Example:
|
Did you forget to commit the I would also need Otherwise looks good! |
Maybe, I was trying to watch Black Mirror at the same time. ¯\_(ツ)_/¯
I was thinking of an API kinda like sql.Driver where each command registers
itself. But I wasn’t quite sure _what_ it should register.
And yeah, those bits (like the stuttering path) are easier to sort out. I
just wanted to see how it’d work and if it was similar to what you were
thinking.
Le sam. 13 janv. 2018 à 07:14, Daniel Martí <[email protected]> a
écrit :
… Did you forget to commit the coreutils package? I'm also not a terrible
fan of the coreutils/coreutils path :) Perhaps you could simply use the
root package, or do something else like coreutils/exec.
I would also need Dir in the context struct, similar to what's in the
os/exec package. Otherwise, the current dir from the process is forced,
which is no good for my interpreter.
Otherwise looks good!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#114 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFnwZ9zKGF9ulmXgadYYtCPxKNIMoisCks5tKMhqgaJpZM4RcIvK>
.
|
Yes, this is similar to what I was thinking. Registering the commands sounds fine. Ping me when there's a working version I can test out :) |
Ok, here's what I meant to commit the other day: 8b35c72 |
Trying it out now, getting this build error on
|
Oh. Just a goofed up build tag inside wc/internal/sys/fadv.go It should be a comma, not a space. Fadv isn't a requirement, anyway. Just theoretically speeds up reading a file by letting the kernel know the desired read pattern. |
Thanks, now it builds. It behaves differently from GNU wc, though. For example, Do you happen to have tests that check input/output of your implementations versus GNU's? |
Also, if you have more time, here's another suggestion to add to the common context - a |
It does? What version of coreutils are you running? Mine's identical with coreutils 8.29.
For some, yeah. I like the |
Simpler example:
Unless I got something very wrong in my prototype, your implementation seems to always include the filename (even if it reads from stdin) and when given no flags, it seems to not print those three numbers. That's what I meant by the examples above. |
Gotcha. One of the goals of this project is to have it be byte-for-byte exact with GNU, but sometimes there are good reasons for it not to be. For example, coreutils is meant to run on VAX and stuff, so there's lots of weird edge-case code and sometimes they go from A -> B -> C -> D to do something that Go (because it can abstract more and doesn't need to support machines from the '80s) can do simply by going from A -> D, if that makes sense. For example, GNU It should be easy enough to make byte-for-byte perfect. |
Thanks - your recent changes make sense. Now my tests almost pass - the only problem is what when reading from stdin it still prints a trailing space, like |
I guess writeCounts will have to wrap its final call to printf inside an fname != “” conditional, then.
Basically there’s two things that need to be done: 1) utils need to be converted from packages to libraries, and 2) any unwritten utils need to be written. Some are easier than others, it’s less daunting than it seems. Perhaps the most annoying part is synthesizing GNU’s source code. I’m fine with doing either.
… On Jan 15, 2018, at 12:19 PM, Daniel Martí ***@***.***> wrote:
Thanks - your recent changes make sense. Now my tests almost pass - the only problem is what when reading from stdin it still prints a trailing space, like wc -c <somefile prints 8 \n. Other than that, all tests should now pass :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Sounds good. Note that I absolutely don't need all the tools at once. In particular, the original issue was just about some of the common ones. This will act as an overlay on top of a real Even if only one or a few tools are importable as libraries, that's plenty for the interpreter to start using them. |
okay. Well, let me know which ones and what you prefer to do, then we can go from there.
Fwiw, some of my tools provide much better performance than GNU’s do :-)
For instance (on my machine, at least!) wc -lmcwL on a 25MB assembly file took 7 seconds for GNU, but something like 0.4 seconds for Go. (I think that benchmark has something to do with the default locale, though.)
… On Jan 15, 2018, at 12:32 PM, Daniel Martí ***@***.***> wrote:
Sounds good. Note that I absolutely don't need all the tools at once. In particular, the original issue was just about some of the common ones. This will act as an overlay on top of a real os/exec call, so on most environments coreutils will be installed and available anyway.
Even if only one or a few tools are importable as libraries, that's plenty for the interpreter to start using them.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Basically any that would be used frequently in shell scripts - |
|
For those of you who saw this thread, I'm trying to coordinate with a different project now :) u-root/u-root#2527 |
Sorry :) |
Certainly not trying to dig up old stuff or put blame - I also have some semi-abandoned projects due to lack of free time and energy :) Just want to point others who might still be interested towards more recent developments. |
Hi Eric! I am developing a shell package - see https://github.com/mvdan/sh.
One of its components is an interpreter. That means I have to implement the shell builtins like
echo
andcd
. One of the big wins of that library is that Go packages that used to needbash
to be installed can simply drop that dependency, and use the shell package as a replacement, statically linked into their binary.However, that breaks down quite easily on systems that don't have coreutils installed. Lots of shell scripts out in the wild depend on coreutils programs like
cat
,rm
andwc
. This is why I opened mvdan/sh#93 - to add them to the interpreter as a sort of second layer of builtins.However, as you probably very well know, adding even just some of them is a ton of work. Which is why I've been looking around for implementations of coreutils.
I could use upstream or the popular implementation in Rust, but that would mean somehow bundling the binaries into the final binary. Something nasty like including them at compile-time as assets and unpacking them into the filesystem at run-time.
But that's not the case with Go, since I can simply import Go packages. Then, the only roadblock that I see is that your tools (nice job, by the way!) are not importable - they are all main packages.
Have you given thought to adding a common interface for all the tools? For example, similar to what
os/exec
does:Then one could do something like
coreutils.Run(Ctx{...}, "wc", "somefile")
.If you have any input, or would like any help to implement this, do let me know.
The text was updated successfully, but these errors were encountered: