Обсуждение: [CF2016-9] Allow spaces in working path on tap-tests
Hello, this is just an entry mail for the next CF. The tap-test fails when the soruce directoy containing spaces. I accidentially hit this by a Jenkins project with the name "test project". The function system_log() is safe for such parameters but backup() uses it in wrong way. On the other hand, enable_restoring() and enable_archiving() forgets to quote such a path containing spaces as already done for Windows. I don't see the similar problem in other places. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
On Mon, Jul 4, 2016 at 4:02 PM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > Hello, this is just an entry mail for the next CF. > > The tap-test fails when the soruce directoy containing spaces. I > accidentially hit this by a Jenkins project with the name "test > project". > > The function system_log() is safe for such parameters but > backup() uses it in wrong way. On the other hand, > enable_restoring() and enable_archiving() forgets to quote such a > path containing spaces as already done for Windows. I don't see > the similar problem in other places. Good catch, your fix looks good to me. I am noticing as well that the invocations of pg_ctl, pg_dumpall, pg_upgrade and diff are likely broken the same way in vcregress.pl. -- Michael
On Mon, Jul 4, 2016 at 4:29 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Mon, Jul 4, 2016 at 4:02 PM, Kyotaro HORIGUCHI > <horiguchi.kyotaro@lab.ntt.co.jp> wrote: >> Hello, this is just an entry mail for the next CF. >> >> The tap-test fails when the soruce directoy containing spaces. I >> accidentially hit this by a Jenkins project with the name "test >> project". >> >> The function system_log() is safe for such parameters but >> backup() uses it in wrong way. On the other hand, >> enable_restoring() and enable_archiving() forgets to quote such a >> path containing spaces as already done for Windows. I don't see >> the similar problem in other places. > > Good catch, your fix looks good to me. I am noticing as well that the > invocations of pg_ctl, pg_dumpall, pg_upgrade and diff are likely > broken the same way in vcregress.pl. And as is the command built for zic.exe in Install.pm, no? $target is normally an absolute path per the call of Install(). -- Michael
On Mon, Jul 4, 2016 at 4:44 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > And as is the command built for zic.exe in Install.pm, no? $target is > normally an absolute path per the call of Install(). Attached is the patch I have in mind. After more investigation zic.exe is indeed broken, $target can be a full path, and if it contains a space things blow up. The commands of vcregress upgradecheck need a cleanup as well. I have merged both patches together and the part for src/tools/msvc needs a backpatch. Separating both things is trivial anyway as the MSVC and the TAP stuff are on two completely separate paths. -- Michael
Вложения
Hello, At Tue, 5 Jul 2016 13:44:08 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqSt2W30tE12eRq7KGB_FPcBpXDX2Zh8XeH2QHFY9Vfb8Q@mail.gmail.com> > On Mon, Jul 4, 2016 at 4:44 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: > > And as is the command built for zic.exe in Install.pm, no? $target is > > normally an absolute path per the call of Install(). > > Attached is the patch I have in mind. After more investigation zic.exe > is indeed broken, $target can be a full path, and if it contains a > space things blow up. The commands of vcregress upgradecheck need a > cleanup as well. I have merged both patches together and the part for > src/tools/msvc needs a backpatch. Separating both things is trivial > anyway as the MSVC and the TAP stuff are on two completely separate > paths. Agreed. Grep'ing "system" in the source tree, I see no more place where needs the same fix. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
On Tue, Jul 5, 2016 at 6:02 PM, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote: > Agreed. Grep'ing "system" in the source tree, I see no more place > where needs the same fix. Same conclusion here. I have added this stuff to the official patch tracker: https://commitfest.postgresql.org/10/663/ I can as well produce patches for back-branches, feel free to ping me if necessary. -- Michael
Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> At Tue, 5 Jul 2016 13:44:08 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in
<CAB7nPqSt2W30tE12eRq7KGB_FPcBpXDX2Zh8XeH2QHFY9Vfb8Q@mail.gmail.com>
>> Attached is the patch I have in mind. After more investigation zic.exe
>> is indeed broken, $target can be a full path, and if it contains a
>> space things blow up. The commands of vcregress upgradecheck need a
>> cleanup as well. I have merged both patches together and the part for
>> src/tools/msvc needs a backpatch. Separating both things is trivial
>> anyway as the MSVC and the TAP stuff are on two completely separate
>> paths.
> Agreed. Grep'ing "system" in the source tree, I see no more place
> where needs the same fix.
This seemed like a bug fix to me, so I went ahead and pushed it. I don't
have any ability to test the Windows parts, so it's possible I missed
something in the back-patching; please review.
regards, tom lane
On Sun, Jul 10, 2016 at 5:50 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > This seemed like a bug fix to me ... Yes, it is. > ... so I went ahead and pushed it. I don't > have any ability to test the Windows parts, so it's possible I missed > something in the back-patching; please review. Thanks! What you have pushed looks fine to me. Also, the portion for src/tools/msvc needs to go further down, I should have precised that earlier. Do you want a patch for that? -- Michael
Michael Paquier <michael.paquier@gmail.com> writes:
> Thanks! What you have pushed looks fine to me. Also, the portion for
> src/tools/msvc needs to go further down, I should have precised that
> earlier. Do you want a patch for that?
Yes, please --- I thought it'd all gotten done.
regards, tom lane
On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paquier@gmail.com> writes: >> Thanks! What you have pushed looks fine to me. Also, the portion for >> src/tools/msvc needs to go further down, I should have precised that >> earlier. Do you want a patch for that? > > Yes, please --- I thought it'd all gotten done. OK, here are patches for 9.1, 9.2 and 9.3. -- Michael
Вложения
Michael Paquier <michael.paquier@gmail.com> writes:
> On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yes, please --- I thought it'd all gotten done.
> OK, here are patches for 9.1, 9.2 and 9.3.
Pushed, thanks.
regards, tom lane