-
-
Notifications
You must be signed in to change notification settings - Fork 236
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
-ldflags=-X with quotes not working when -literals is also used #492
Comments
Hmm, is that kind of nested quoting right? I don't think the linker knows about it, so it just discards the malformed This works:
This does not, whether or not garble is involved:
|
hmm, I got a different result
|
Could you try a garble build without the quoting, like I did? Also, is it at all possible that you're not actually rebuilding merlin.bin with |
Removing the quotes does work. I'll need quotes to set multiple
Is it possible there is some type of caching that is saving a "good" run that is re-used? I added the
|
I'm still not sure why we're seeing inconsistent results, but I'm pretty certain that neither |
That sounds right, I already set multiple |
You are absolutely right; And I totally botched my examples earlier, as you point out. I did not use |
I can also confirm there's something funky with build caching. I get the expected results with regards to quoting if I add the |
In particular, using -ldflags with - In particular, a command like: garble -literals build -ldflags='-X "main.foo=foo bar"' would fail, because we would try to use "\"main" as the package name for the -X qualified name, with the leading quote character. This is because we used strings.Split(ldflags, " "). Instead, use the same quoted.Split that cmd/go uses, copied over thanks to x/tools/cmd/bundle and go:generate. Updates burrowers#492.
Please test the PR above when you can :) I'll leave this PR open to investigate the cache issue further. |
I'm not sure which is right? The difference is a
|
I also noticed the quoting does not work with the extra equals sign. This seems to be the same both with and without garble. |
Can confirm that the associate PR enables garble to work with the quoted ldflags arguments as the way that I currently use it when calling go directly. |
Thanks for confirming! |
In particular, using -ldflags with - In particular, a command like: garble -literals build -ldflags='-X "main.foo=foo bar"' would fail, because we would try to use "\"main" as the package name for the -X qualified name, with the leading quote character. This is because we used strings.Split(ldflags, " "). Instead, use the same quoted.Split that cmd/go uses, copied over thanks to x/tools/cmd/bundle and go:generate. Updates burrowers#492.
In particular, using -ldflags with - In particular, a command like: garble -literals build -ldflags='-X "main.foo=foo bar"' would fail, because we would try to use "\"main" as the package name for the -X qualified name, with the leading quote character. This is because we used strings.Split(ldflags, " "). Instead, use the same quoted.Split that cmd/go uses, copied over thanks to x/tools/cmd/bundle and go:generate. Updates #492.
Noticed while debugging burrowers#492, as adding -debug to a previously run command would unexpectedly rebuild all packages in the build, as if -a was given. While here, remove commented out testscript that were kept in error.
Noticed while debugging #492, as adding -debug to a previously run command would unexpectedly rebuild all packages in the build, as if -a was given. While here, remove commented out testscript that were kept in error.
I spent some time trying to reproduce the bug but couldn't, so for now, make a detailed note for it. We can come back to it if we actually run into it in the future. Fixes burrowers#492.
I couldn't reproduce the cache issue, even though I still believe it exists by inspecting our code. The PR above closes this issue by adding a TODO. |
I spent some time trying to reproduce the bug but couldn't, so for now, make a detailed note for it. We can come back to it if we actually run into it in the future. Fixes #492.
My fault... I write the package name in a wrong way... |
What version of Garble and Go are you using?
What environment are you running Garble on?
go env
OutputWhat did you do?
I compiled Merlin using Garble with the
-literals
argument and attempted to setverbose
variable through the go buildldflags
argument. When I ran the agent, there was no output to STDOUT but there should have been if theverbose
variable was set.What did you expect to see?
I expected to see verbose output to STDOUT because the
verbose
variable was set to True. The output would look like:What did you see instead?
I saw no output, indicating that the
verbose
variable was not set.Misc.
I opened
main.go
and addedfunc init(){ fmt.Printf("Verbose: %s\n", verbose) }
to assist with debugging:Potentially related to #323
The text was updated successfully, but these errors were encountered: