On Thu, Apr 12, 2018 at 6:29 PM, Josh Soref <jsoref at gmail.com> wrote:> Randall S. Becker <rsbecker at nexbridge.com> wrote: > >> >> -REGRESSTMP = "$(PWD)/regress" >> +REGRESSTMP = `pwd` >> >> tests interop-tests t-exec unit: regress-prep regress-binaries $(TARGETS) >> > > It looks like the problem is that pwd is in uppercase, not so much the > distinction between $() and ``.PWD is a commonly set environment variable, built into shells such as bash, Changing it from "$(PWD)/regress" to `pwd` is begging for issues compiling it in Windows environments, where spaces and Unicode may be in the working directory name.. It's also no longer setting the directory to the "/regress" subdirectory, which is begging for confusion if done uncautiously.
On April 12, 2018 9:41 PM, Nico Kadel-Garcia wrote:> On Thu, Apr 12, 2018 at 6:29 PM, Josh Soref <jsoref at gmail.com> wrote: > > Randall S. Becker <rsbecker at nexbridge.com> wrote: > > > >> > >> -REGRESSTMP = "$(PWD)/regress" > >> +REGRESSTMP = `pwd` > >> > >> tests interop-tests t-exec unit: regress-prep regress-binaries > >> $(TARGETS) > >> > > > > It looks like the problem is that pwd is in uppercase, not so much the > > distinction between $() and ``. > > PWD is a commonly set environment variable, built into shells such asbash,> Changing it from "$(PWD)/regress" to `pwd` is begging for issues compilingit> in Windows environments, where spaces and Unicode may be in the working > directory name.. It's also no longer setting the directory to the"/regress"> subdirectory, which is begging for confusion if done uncautiously.The Makefile has large numbers of references to `pwd`. I would respectfully suggest that this particular objection would require a more invasive bit of work. The original test issue, however, was that the test Makefile was receiving a value of only '/regress' when driven by bash and both gmake and make. Putting it in quotes may be fine one Windows, but it broke the test suite. 7.6p1 tested fine, so I have to assume the breakage was there, especially when patched. Cheers, Randall
On Thu, Apr 12, 2018 at 10:24 PM, Randall S. Becker <rsbecker at nexbridge.com> wrote:> On April 12, 2018 9:41 PM, Nico Kadel-Garcia wrote: >> On Thu, Apr 12, 2018 at 6:29 PM, Josh Soref <jsoref at gmail.com> wrote: >> > Randall S. Becker <rsbecker at nexbridge.com> wrote: >> > >> >> >> >> -REGRESSTMP = "$(PWD)/regress" >> >> +REGRESSTMP = `pwd` >> >> >> >> tests interop-tests t-exec unit: regress-prep regress-binaries >> >> $(TARGETS) >> >> >> > >> > It looks like the problem is that pwd is in uppercase, not so much the >> > distinction between $() and ``. >> >> PWD is a commonly set environment variable, built into shells such as > bash, >> Changing it from "$(PWD)/regress" to `pwd` is begging for issues compiling > it >> in Windows environments, where spaces and Unicode may be in the working >> directory name.. It's also no longer setting the directory to the > "/regress" >> subdirectory, which is begging for confusion if done uncautiously. > > The Makefile has large numbers of references to `pwd`. I would respectfully > suggest that this particular objection would require a more invasive bit of > work. The original test issue, however, was that the test Makefile was > receiving a value of only '/regress' when driven by bash and both gmake and > make. Putting it in quotes may be fine one Windows, but it broke the test > suite. 7.6p1 tested fine, so I have to assume the breakage was there, > especially when patched.Part of my concern was that changing from "$(PWD)/regress" to `pwd` strips off the double quote wrapper. It's really easy to alter behavior, by accident, that will bite someone else in a different environment.. You also have my sympathies. I went through some of this with gnu "make" and PWD recently, due to using this syntax: PWD = `/bin/pwd` Hilarity ensued when embedding this inside *other* scripting in the Makefile.