Shell macros are broken in recent master, e. g. "$(dirname %s)"

[davido]

Before this commit d6f3252 something like this was valid:

def custom(
    name,
    dest):
  genrule(
    name = name,
    cmd = 'cd $TMP;' +
      ('mkdir -p $(dirname %s);' % '$OUT') +
       'echo "__done__" >$OUT',
    out = dest
  )

custom(
  name = 'foo',
  dest = 'bar/baz/qux.txt',
)

With the resolution:

$ buck targets --json :foo
Using buckd.
[
{
  "buck.base_path" : "",
  "buck.output_file" : "buck-out/gen/bar/baz/qux.txt",
  "cmd" : "cd $TMP;mkdir -p $(dirname $OUT);echo \"done\" >$OUT",
  "deps" : [ ],
  "name" : "foo",
  "out" : "bar/baz/qux.txt",
  "srcs" : [ ],
  "type" : "genrule",
  "visibility" : [ ]
}
]

In fact Gerrit Code Review project is relying on this "feature" in its toolchain.

After the mentioned commit, this Buck sniplet is rejected with:

$ cat .buckversion
d6f3252170abe7a6de8e7dec0cc90000147c5d10

$ buck build :foo
Using buckd.
BUILD FAILED: //:foo: no such macro "dirname"

[andrewjcg]

Since we use the "$(macro ...)" syntax for buck macros, you need to use a backslash to send this directly to the shell: "$(macro ...)". Does using "$(dirname ...)" work?

[andrewjcg]

I'll update the documentation regarding this, which I should of done when we initially pushed this.

[andrewjcg]

And sorry for the disruption here -- we don't see a lot of this style of sub-shelling internally.

[andrewjcg]

Also, FYI, shell backticks are safe to use before and after this change (e.g. dirname ...).

[davido]

I was trying shell backticks: it was failing with scary error messages. I probably did something wrong.

[davido]

Does using "$(dirname ...)" work?

Yes, that fixed it. Thanks. Documening this would be nice, though.

[davido]

Opps. It actually didn't fix it:

genrule(
  name = 'ui_optdbg',
  cmd = 'cd $TMP;' +
    'unzip -q $(location :ui_dbg);' +
    'mv' +
    ' gerrit_ui/gerrit_ui.nocache.js' +
    ' gerrit_ui/dbg_gerrit_ui.nocache.js;' +
    'unzip -qo $(location :ui_opt);' +
    'mkdir -p \$(dirname $OUT);' +
    'zip -qr $OUT .',
  deps = [
    ':ui_dbg',
    ':ui_opt',
  ],
  out = 'ui_optdbg.zip',
  visibility = ['PUBLIC'],
)

With this resolution

[
{
  "bash" : null,
  "buck.base_path" : "gerrit-gwtui",
  "buck.output_file" : "buck-out/gen/gerrit-gwtui/ui_optdbg.zip",
  "cmd" : "cd $TMP;unzip -q $(location :ui_dbg);mv gerrit_ui/gerrit_ui.nocache.js gerrit_ui/dbg_gerrit_ui.nocache.js;unzip -qo $(location :ui_opt);mkdir -p \\$(dirname $OUT);zip -qr $OUT .",
  "cmdExe" : null,
  "deps" : [ ":ui_dbg", ":ui_opt" ],
  "name" : "ui_optdbg",
  "out" : "ui_optdbg.zip",
  "srcs" : [ ],
  "type" : "genrule",
  "visibility" : [ "PUBLIC" ]
}
]

And with this error:

$ buck build gerrit-gwtui:ui_optdbg
Using buckd.
[-] PROCESSING BUCK FILES...FINISHED 0,0s
[+] BUILDING...4,4s (85/87 JOBS)
 |=> //gerrit-gwtui:ui_opt...  2,4s (checking local cache)
/bin/bash: -c: line 0: syntax error near unexpected token `('
BUILD FAILED: //gerrit-gwtui:ui_optdbg failed with exit code 2:
genrule

[davido]

OK, shell backticks work.

[andrewjcg]

Changes to fix the backslash behavior and add documentation have been pushed internally. We'll aim to do an open-source push soon.

[davido]

Thanks. We are facing another side effect here. We are still supporting Maven archetypes for plugin skeleton generation. As a part of the generated plugin we generate Buck files. In raw form they are looking something like this:

include_defs('//bucklets/gerrit_plugin.bucklet')

gerrit_plugin(
  name = '${pluginName}',
  srcs = glob(['src/main/java/**/*.java']),
  resources = glob(['src/main/resources/**/*']),
  manifest_entries = [
    'Gerrit-PluginName: ${pluginName}',
    'Gerrit-ApiType: ${gerritApiType}',
    'Gerrit-ApiVersion: ${gerritApiVersion}',
    'Gerrit-Module: ${package}.Module',
    'Gerrit-SshModule: ${package}.SshModule',
    'Gerrit-HttpModule: ${package}.HttpModule',
  ],
)

java_library(
  name = 'classpath',
  deps = [':${pluginName}__plugin'],
)

Note: that all ${foo} are replaced by Maven. After this change Buck rejects all this places with macro error. See this change fo more details [1].

[1] https://gerrit-review.googlesource.com/#/c/61074/4/gerrit-plugin-archetype/src/main/resources/archetype-resources/BUCK

[andrewjcg]

https://gist.github.com/andrewjcg/0b9b18a5b036784092e3 got this working for me.

[andrewjcg]

Err, that gist is dependent on the fixes committed internally (but backticks should work in the meantime).

[davido]

https://gist.github.com/andrewjcg/0b9b18a5b036784092e3 got this working for me.

I think it interferes also with ${pluginName} constructs.

[andrewjcg]

Hmm, it shouldn't be. I tested the ${something} style locally and the macro expander doesn't try to expand it. Are you seeing an error other than the "git" one that makes you think this is causing issues?

[davido]

Sorry, my fault. It was failing because of git describe that was fixed by you in the gist. Thanks for doing my job ;-) I ammended the Buck upgrade change and all seems to be fine. Thanks again for your help.

[davido]

@andrewjcg Thanks for quick fix. After upgrading to recent master we were able to eliminate shell back ticks again and switch to escaped macro invocation:

  \$(dirname $OUT)

[1] https://gerrit-review.googlesource.com/61204