-
Notifications
You must be signed in to change notification settings - Fork 894
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
Allow read data from stdin on Windows #1104
Conversation
In case tests are missing, please give me a hint where should I add the tests, thanks. |
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.
Seems mostly OK for me. A few improvements needed now that error message is managed centrally.
How to move forward here? |
@ajvb (chosen as an active maintainer) could you take a look here? |
@ajvb any change of taking a look and merging? This is a nice fix that resolves having to use a custom build of SOPS on Windows |
Changing merge target to develop |
Reading #927, its like it will take some time. |
@@ -441,3 +441,19 @@ func PrettyPrintDiffs(diffs []Diff) { | |||
} | |||
} | |||
} | |||
|
|||
func ReadFile(path string) (content []byte, err error) { |
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.
I think this tries to do two things which really should be separate things.
In my opinion, handling of files should really be just handling of files, with os.Stdin
not classifying as such. Part because from the perspective of SOPS as an SDK, decrypt.Decrypt
magically reading from os.Stdin
would be kind of surprising.
Having said this, I would deal with -
as a special path case within commands, and e.g. create another wrapper around DataWithFormat
for an io.Reader
.
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.
files should really be just handling of files
I don't know. On Unix, stdin is just an other file descriptor and could be handle as file, too. (like /dev/stdin
)
Living the unix philosophy, there should not be a difference between /mnt/file
and /proc/self/fd/1
.
From technical point of view, it may should fine.
Creating an other wrapper, it possible. But the other wrapper would be the exact same copy/paste code.
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.
How is this the same copy/paste code?
func Reader(r io.Reader, format string) (cleartext []byte, err error) {
encryptedData, err := io.ReadAll(r)
if err != nil {
return nil, err
}
return DataWithFormat(encryptedData, FormatFromString(format))
}
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.
NB: I am fine with a function called ReadContent
(or something something) existing (and utilized) within cmd/sops
, but I wouldn't want this to be the default for decrypt
.
Fixes #739
Allow
-
as cross platform alternative to/dev/stdin
for reading data from STDIN.