[PATCH] D29677: [buildbot] Add check-fuzzer to Asan buildbot on Windows.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: zorg/buildbot/builders/SanitizerBuilderWindows.py:140
+    # Get binary dir.
+    bin_path = "\\".join([Property("builddir"), config, "bin"])
+    # Get compiler-rt's libraries dir.
----------------
mpividori wrote:
> rnk wrote:
> > I doubt this will work. A "Property" is not a string or dictionary. The master never sees the property values until the build actually executes. It has to generate the list of steps to execute up front, before actually running any of them. When you say "addStep", that adds a step to be run in the future on another machine. The steps don't run synchronously inside this python code.
> > 
> > As an alternative, we don't need to set PATH if we link statically, right? I'd recommend we do that. Set "-DLLVM_USE_CRT_"+config.upper()"=MT" in the cmake command to do that.
> @rnk Thanks. We also need to set PATH because we need to add `Filecheck`, `not`, `sancov` to PATH.
True, but we don't have to compute the clang version to compute the path the bin directory. Static linking would still eliminate the need for this "dir /b" step.


================
Comment at: zorg/buildbot/builders/SanitizerBuilderWindows.py:143
+    # Get binary dir.
+    bin_path = "%(prop:builddir)s\\" + config + "\\bin"
+    # Get absolute path to clang-cl.
----------------
Why add config? A ninja build will put the binaries in "%(builddir)s/bin", right?


================
Comment at: zorg/buildbot/builders/SanitizerBuilderWindows.py:148
+    # Update slave_env to add fresh clang, tools and compiler-rt dlls to path.
+    update_path_cmd = "set Path=\""+bin_path+";"+dll_path+";%Path%\" && set"
+    f.addStep(SetProperty(command=Interpolate(update_path_cmd),
----------------
This will probably work, nice :)


https://reviews.llvm.org/D29677