Remove go-homedir dependency #1333
Conversation
justincormack
force-pushed
the
no-homedir
branch
from
36cce4f
to
86c7d9f
Compare
cmd/notary/util.go
Outdated
| // homeExpand will expand an initial ~ to the user home directory. This is supported for | ||
| // config files where the shell will not have expanded paths. | ||
| func homeExpand(homeDir, path string) string { | ||
| if path == "" || path[1] != '~' || (len(path) > 1 && path[1] != os.PathSeparator) { |
There was a problem hiding this comment.
Should path[1] != '~' be path[0] ~= '~'? Since we only want to expand if the first char is ~? (e.g. ~/.docker).
Also, how would we expand for something like ~cyli or ~justincormack? That is not the homedir of the current user, necessarily.
There was a problem hiding this comment.
ah yes, typo, fixed now.
This has never worked with ~cyli etc so there is no change here. If you put that on the command line your shell will expand it before notary sees it anyway, so it is only for use in the config file, where it would be quite odd to put files in another user's home.
There was a problem hiding this comment.
This has never worked with ~cyli etc so there is no change here. If you put that on the command line your shell will expand it before notary sees it anyway, so it is only for use in the config file, where it would be quite odd to put files in another user's home.
Ah ok, I didn't realize that library never expanded that correctly. But makes sense that we shouldn't put that in the config file.
justincormack
force-pushed
the
no-homedir
branch
2 times, most recently
from
6d4451a
to
2141477
Compare
|
I should add some tests to clarify this.
…On Mon, 9 Apr 2018, 18:22 Ying Li, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cmd/notary/util.go
<#1333 (comment)>
:
> @@ -52,3 +53,12 @@ func feedback(t *tufCommander, payload []byte) error {
os.Stdout.Write(payload)
return nil
}
+
+// homeExpand will expand an initial ~ to the user home directory. This is supported for
+// config files where the shell will not have expanded paths.
+func homeExpand(homeDir, path string) string {
+ if path == "" || path[1] != '~' || (len(path) > 1 && path[1] != os.PathSeparator) {
This has never worked with ~cyli etc so there is no change here. If you
put that on the command line your shell will expand it before notary sees
it anyway, so it is only for use in the config file, where it would be
quite odd to put files in another user's home.
Ah ok, I didn't realize that library never expanded that correctly. But
makes sense that we shouldn't put that in the config file.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1333 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdcPJNxzGkCIoVb2MdZIm7ezVAXKjUvks5tm5jegaJpZM4TLNoL>
.
|
That would be awesome, thanks. I was wondering if we had tests to make sure |
This is overcomplicated for a very simple requirement. It can call `exec`, it has a cache protected by a mutex because it is so complex. Just use the per platform environment variables which will always be set, if the user has a weird environment they can specify full paths. Signed-off-by: Justin Cormack <justin.cormack@docker.com>
justincormack
force-pushed
the
no-homedir
branch
from
2141477
to
de8cd46
Compare
|
Added a unit test for |
This is overcomplicated for a very simple requirement. It can call
exec, it has a cache protected by a mutex because it is so complex.Just use the per platform environment variables which will always be
set, if the user has a weird environment they can specify full paths.
Signed-off-by: Justin Cormack justin.cormack@docker.com