Thread: forward vs backward slashes in msvc build code
I understand that on Windows, you can use forward slashes in path names interchangeably with backward slashes (except possibly in the shell). I have developed the attached patch to change the msvc build code to use forward slashes consistently. Together with the other attached patch, which is an unpolished hack, this allows me to run the build.pl script on not-Windows. It won't actually build, but it will create all the project files. That way, I can check whether some makefile hackery would break the Windows build. Could someone verify this please?
Attachment
On Sun, Feb 15, 2015 at 12:51 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > I understand that on Windows, you can use forward slashes in path names > interchangeably with backward slashes (except possibly in the shell). I > have developed the attached patch to change the msvc build code to use > forward slashes consistently. Together with the other attached patch, > which is an unpolished hack, this allows me to run the build.pl script > on not-Windows. It won't actually build, but it will create all the > project files. That way, I can check whether some makefile hackery > would break the Windows build. This sounds like a good idea to improve the checks of the scripts of Windows, like the presence of win32rc files, etc. > Could someone verify this please? I tested quickly the second patch with MS 2010 and I am getting a build failure: chkpass cannot complete because of crypt missing. On master build passes. More details here: "C:\Users\mpaquier\git\postgres\pgsql.sln" (default target) (1) -> "C:\Users\mpaquier\git\postgres\chkpass.vcxproj" (default target) (36) -> (Link target) -> chkpass.obj : error LNK2019: unresolved external symbol crypt referenced in function chkpass_in [C:\Users\ioltas\git\postgres\chkpass.vcxproj] .\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj] 0 Warning(s) 2 Error(s) Regards, -- Michael
On 2/15/15 6:55 AM, Michael Paquier wrote: > I tested quickly the second patch with MS 2010 and I am getting a > build failure: chkpass cannot complete because of crypt missing. On > master build passes. More details here: > "C:\Users\mpaquier\git\postgres\pgsql.sln" (default target) (1) -> > "C:\Users\mpaquier\git\postgres\chkpass.vcxproj" (default target) (36) -> > (Link target) -> > chkpass.obj : error LNK2019: unresolved external symbol crypt > referenced in function chkpass_in > [C:\Users\ioltas\git\postgres\chkpass.vcxproj] > .\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved > externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj] I can't tell from just looking at the code how chkpass would normally find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded knowledge. Any idea?
On 03/04/2015 10:37 PM, Peter Eisentraut wrote: > On 2/15/15 6:55 AM, Michael Paquier wrote: >> I tested quickly the second patch with MS 2010 and I am getting a >> build failure: chkpass cannot complete because of crypt missing. On >> master build passes. More details here: >> "C:\Users\mpaquier\git\postgres\pgsql.sln" (default target) (1) -> >> "C:\Users\mpaquier\git\postgres\chkpass.vcxproj" (default target) (36) -> >> (Link target) -> >> chkpass.obj : error LNK2019: unresolved external symbol crypt >> referenced in function chkpass_in >> [C:\Users\ioltas\git\postgres\chkpass.vcxproj] >> .\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved >> externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj] > I can't tell from just looking at the code how chkpass would normally > find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded > knowledge. Any idea? > Which library is it in? There are sections at the top of Mkvcbuild.pm for including various libraries in contrib modules that need them. cheers andrew
On 3/4/15 11:00 PM, Andrew Dunstan wrote: > > On 03/04/2015 10:37 PM, Peter Eisentraut wrote: >> On 2/15/15 6:55 AM, Michael Paquier wrote: >>> I tested quickly the second patch with MS 2010 and I am getting a >>> build failure: chkpass cannot complete because of crypt missing. On >>> master build passes. More details here: >>> "C:\Users\mpaquier\git\postgres\pgsql.sln" (default target) (1) -> >>> "C:\Users\mpaquier\git\postgres\chkpass.vcxproj" (default target) >>> (36) -> >>> (Link target) -> >>> chkpass.obj : error LNK2019: unresolved external symbol crypt >>> referenced in function chkpass_in >>> [C:\Users\ioltas\git\postgres\chkpass.vcxproj] >>> .\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved >>> externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj] >> I can't tell from just looking at the code how chkpass would normally >> find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded >> knowledge. Any idea? > Which library is it in? There are sections at the top of Mkvcbuild.pm > for including various libraries in contrib modules that need them. This is contrib/chkpass not finding the crypt symbol, which is presumably in some library. But I can't see how it would normally find it, without my patch.
Peter Eisentraut wrote: > On 3/4/15 11:00 PM, Andrew Dunstan wrote: > > > > On 03/04/2015 10:37 PM, Peter Eisentraut wrote: > >> I can't tell from just looking at the code how chkpass would normally > >> find crypt. The msvc tools neither parse SHLIB_LINK nor have hardcoded > >> knowledge. Any idea? > > > Which library is it in? There are sections at the top of Mkvcbuild.pm > > for including various libraries in contrib modules that need them. > > This is contrib/chkpass not finding the crypt symbol, which is > presumably in some library. But I can't see how it would normally find > it, without my patch. It seems crypt is provided by libpgport. So chkpass should be mentioned in @contrib_uselibpgport, but isn't. Maybe the fix is just to add it there? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Mar 13, 2015 at 6:20 AM, Alvaro Herrera wrote: > Peter Eisentraut wrote: >> This is contrib/chkpass not finding the crypt symbol, which is >> presumably in some library. But I can't see how it would normally find >> it, without my patch. > > It seems crypt is provided by libpgport. So chkpass should be mentioned > in @contrib_uselibpgport, but isn't. Maybe the fix is just to add it > there? I had a closer look at this patch, and yes indeed, the problem was exactly that. Now honestly I cannot understand why this dependency with libpgport was not necessary before... In any case, attached is a patch rebased on HEAD that builds correctly with MSVC. -- Michael
Attachment
On Fri, Mar 13, 2015 at 6:04 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Fri, Mar 13, 2015 at 6:20 AM, Alvaro Herrera wrote: >> Peter Eisentraut wrote: >>> This is contrib/chkpass not finding the crypt symbol, which is >>> presumably in some library. But I can't see how it would normally find >>> it, without my patch. >> >> It seems crypt is provided by libpgport. So chkpass should be mentioned >> in @contrib_uselibpgport, but isn't. Maybe the fix is just to add it >> there? > > I had a closer look at this patch, and yes indeed, the problem was > exactly that. Now honestly I cannot understand why this dependency > with libpgport was not necessary before... In any case, attached is a > patch rebased on HEAD that builds correctly with MSVC. Now that the stuff related to the move from contrib/ to src/bin/, modulescheck and tmp_install has been committed, shouldn't we give a new shot at this patch? Attached is a rebased version. -- Michael
Attachment
On 4/24/15 12:22 AM, Michael Paquier wrote: > On Fri, Mar 13, 2015 at 6:04 PM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> On Fri, Mar 13, 2015 at 6:20 AM, Alvaro Herrera wrote: >>> Peter Eisentraut wrote: >>>> This is contrib/chkpass not finding the crypt symbol, which is >>>> presumably in some library. But I can't see how it would normally find >>>> it, without my patch. >>> >>> It seems crypt is provided by libpgport. So chkpass should be mentioned >>> in @contrib_uselibpgport, but isn't. Maybe the fix is just to add it >>> there? >> >> I had a closer look at this patch, and yes indeed, the problem was >> exactly that. Now honestly I cannot understand why this dependency >> with libpgport was not necessary before... In any case, attached is a >> patch rebased on HEAD that builds correctly with MSVC. > > Now that the stuff related to the move from contrib/ to src/bin/, > modulescheck and tmp_install has been committed, shouldn't we give a > new shot at this patch? Attached is a rebased version. done
On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote: > On 4/24/15 12:22 AM, Michael Paquier wrote: >> Now that the stuff related to the move from contrib/ to src/bin/, >> modulescheck and tmp_install has been committed, shouldn't we give a >> new shot at this patch? Attached is a rebased version. > > done Thanks, bowerbird is running fine: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2015-04-25%2013%3A31%3A06 -- Michael
On 04/25/2015 10:53 AM, Michael Paquier wrote: > On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote: >> On 4/24/15 12:22 AM, Michael Paquier wrote: >>> Now that the stuff related to the move from contrib/ to src/bin/, >>> modulescheck and tmp_install has been committed, shouldn't we give a >>> new shot at this patch? Attached is a rebased version. >> done > Thanks, bowerbird is running fine: > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2015-04-25%2013%3A31%3A06 But currawong is not - it runs an older version of the MS build tools. See <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-25%2016%3A31%3A12>: Bad format filename 'src/backend/access/brin/brin.c' at src/tools/msvc/VCBuildProject.pm line 73. VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274),"*Project::F") called at src/tools/msvc/Project.pm line 363 Project::Save(VC2008Project=HASH(0x9a7274)) called at src/tools/msvc/Solution.pm line 539 Solution::Save(VS2008Solution=HASH(0xc00b7c))called at src/tools/msvc/Mkvcbuild.pm line 656 Mkvcbuild::mkvcbuild(HASH(0xbed464))called at build.pl line 36 cheers andrew
On Sun, Apr 26, 2015 at 2:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 04/25/2015 10:53 AM, Michael Paquier wrote: >> >> On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote: >>> >>> On 4/24/15 12:22 AM, Michael Paquier wrote: >>>> >>>> Now that the stuff related to the move from contrib/ to src/bin/, >>>> modulescheck and tmp_install has been committed, shouldn't we give a >>>> new shot at this patch? Attached is a rebased version. >>> >>> done >> >> Thanks, bowerbird is running fine: >> >> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2015-04-25%2013%3A31%3A06 > > > > But currawong is not - it runs an older version of the MS build tools. See > <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-25%2016%3A31%3A12>: > > Bad format filename 'src/backend/access/brin/brin.c' > at src/tools/msvc/VCBuildProject.pm line 73. > VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274), > "*Project::F") called at src/tools/msvc/Project.pm line 363 > Project::Save(VC2008Project=HASH(0x9a7274)) called at > src/tools/msvc/Solution.pm line 539 > Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at > src/tools/msvc/Mkvcbuild.pm line 656 > Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36 Oops, attached is a patch that should make currawong happier.. -- Michael
Attachment
On 04/25/2015 08:01 PM, Michael Paquier wrote: > On Sun, Apr 26, 2015 at 2:19 AM, Andrew Dunstan <andrew@dunslane.net> wrote: >> On 04/25/2015 10:53 AM, Michael Paquier wrote: >>> On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote: >>>> On 4/24/15 12:22 AM, Michael Paquier wrote: >>>>> Now that the stuff related to the move from contrib/ to src/bin/, >>>>> modulescheck and tmp_install has been committed, shouldn't we give a >>>>> new shot at this patch? Attached is a rebased version. >>>> done >>> Thanks, bowerbird is running fine: >>> >>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2015-04-25%2013%3A31%3A06 >> >> >> But currawong is not - it runs an older version of the MS build tools. See >> <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-25%2016%3A31%3A12>: >> >> Bad format filename 'src/backend/access/brin/brin.c' >> at src/tools/msvc/VCBuildProject.pm line 73. >> VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274), >> "*Project::F") called at src/tools/msvc/Project.pm line 363 >> Project::Save(VC2008Project=HASH(0x9a7274)) called at >> src/tools/msvc/Solution.pm line 539 >> Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at >> src/tools/msvc/Mkvcbuild.pm line 656 >> Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36 > Oops, attached is a patch that should make currawong happier.. OK, pushed. cheers andrew
On Sun, Apr 26, 2015 at 10:29 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 04/25/2015 08:01 PM, Michael Paquier wrote: >> >> On Sun, Apr 26, 2015 at 2:19 AM, Andrew Dunstan <andrew@dunslane.net> >> wrote: >>> >>> On 04/25/2015 10:53 AM, Michael Paquier wrote: >>>> >>>> On Sat, Apr 25, 2015 at 9:58 PM, Peter Eisentraut wrote: >>>>> >>>>> On 4/24/15 12:22 AM, Michael Paquier wrote: >>>>>> >>>>>> Now that the stuff related to the move from contrib/ to src/bin/, >>>>>> modulescheck and tmp_install has been committed, shouldn't we give a >>>>>> new shot at this patch? Attached is a rebased version. >>>>> >>>>> done >>>> >>>> Thanks, bowerbird is running fine: >>>> >>>> >>>> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bowerbird&dt=2015-04-25%2013%3A31%3A06 >>> >>> >>> >>> But currawong is not - it runs an older version of the MS build tools. >>> See >>> >>> <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-25%2016%3A31%3A12>: >>> >>> Bad format filename 'src/backend/access/brin/brin.c' >>> at src/tools/msvc/VCBuildProject.pm line 73. >>> VCBuildProject::WriteFiles(VC2008Project=HASH(0x9a7274), >>> "*Project::F") called at src/tools/msvc/Project.pm line 363 >>> Project::Save(VC2008Project=HASH(0x9a7274)) called at >>> src/tools/msvc/Solution.pm line 539 >>> Solution::Save(VS2008Solution=HASH(0xc00b7c)) called at >>> src/tools/msvc/Mkvcbuild.pm line 656 >>> Mkvcbuild::mkvcbuild(HASH(0xbed464)) called at build.pl line 36 >> >> Oops, attached is a patch that should make currawong happier.. > > > OK, pushed. Argh. This is not enough: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-26%2006%3A00%3A01 The build is done in Debug mode, but it is failing to find some files under the label Release, which is incorrect. I guess that this is caused by the file detection in WriteFiles... TBH I don't have an environment at hand to reproduce the issue and do a proper fix. Hence I think that we should revert the patch for now, and come back to this stuff later on. Regards, -- Michael
On 04/26/2015 03:13 AM, Michael Paquier wrote: >>> >>> Oops, attached is a patch that should make currawong happier.. >> >> OK, pushed. > Argh. This is not enough: > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=currawong&dt=2015-04-26%2006%3A00%3A01 > The build is done in Debug mode, but it is failing to find some files > under the label Release, which is incorrect. I guess that this is > caused by the file detection in WriteFiles... TBH I don't have an > environment at hand to reproduce the issue and do a proper fix. Hence > I think that we should revert the patch for now, and come back to this > stuff later on. Well, currawong is mine. so I can try anything you want to suggest. But I don't think it's set up to build Debug. Setting up a VS2008 environment is hard these days, as MS seems to have removed the download :-( cheers andrew
On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote: > Setting up a VS2008 environment is hard these days, as MS seems to have > removed the download :-( Visual C++ 2008 Express Edition: http://go.microsoft.com/?linkid=7729279
On 04/26/2015 04:08 PM, Noah Misch wrote: > On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote: >> Setting up a VS2008 environment is hard these days, as MS seems to have >> removed the download :-( > Visual C++ 2008 Express Edition: > http://go.microsoft.com/?linkid=7729279 > Oh, cool. Thanks. cheers andrew
On Mon, Apr 27, 2015 at 8:53 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 04/26/2015 04:08 PM, Noah Misch wrote: >> >> On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote: >>> >>> Setting up a VS2008 environment is hard these days, as MS seems to have >>> removed the download :-( >> >> Visual C++ 2008 Express Edition: >> http://go.microsoft.com/?linkid=7729279 >> > > Oh, cool. Thanks. Thanks, I'll try to set up an environment and come up with a real patch tested this time. What I suspect is that we need to correctly replace all the references to backslaches like '\\' or $obj =~ s/\\/_/g; to make things work correctly. Regards, -- Michael
On Mon, Apr 27, 2015 at 9:20 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Mon, Apr 27, 2015 at 8:53 AM, Andrew Dunstan <andrew@dunslane.net> wrote: >> >> On 04/26/2015 04:08 PM, Noah Misch wrote: >>> >>> On Sun, Apr 26, 2015 at 10:23:58AM -0400, Andrew Dunstan wrote: >>>> >>>> Setting up a VS2008 environment is hard these days, as MS seems to have >>>> removed the download :-( >>> >>> Visual C++ 2008 Express Edition: >>> http://go.microsoft.com/?linkid=7729279 >>> >> >> Oh, cool. Thanks. > > Thanks, I'll try to set up an environment and come up with a real > patch tested this time. > > What I suspect is that we need to correctly replace all the references > to backslaches like '\\' or $obj =~ s/\\/_/g; to make things work > correctly. So, I have set up an environment with VC tools, and have been able to reproduce the failure. The problems were less trivial than I though first: 1) - The file tree list was not correctly generated, build script generating vcproj file missing tree dependencies when listing items in Filter. For example, for a path like src/backend/access, it missed to list "src", "backend" and "access", listing only the files in such paths. 2) I noticed that VC builds do not like *at all* file paths with forward slashes, perhaps it could be possible to use a Condition like hasForwardSlashes but it seems safer to simply enforce the file paths to use backslashes in the vcproj files. That's inconsistent with the vcxproj files, but it works and Mkvcbuild.pm works as before. 3) I found why chkpass was needed as a dependency with libpgport and libpgcommon, actually crypt.c needs to be listed as a unique file, so it needs a fake entry in vcproj and vcxproj files. All those things are corrected in the patch attached, tested with both MS and VC. The build is still failing because of cac76582 that introduced the transforms, but at least it will partially cure currawong and put it at the same level as the other machines. Regards, -- Michael
Attachment
On 4/27/15 2:09 AM, Michael Paquier wrote: > 2) I noticed that VC builds do not like *at all* file paths with > forward slashes, perhaps it could be possible to use a Condition like > hasForwardSlashes but it seems safer to simply enforce the file paths > to use backslashes in the vcproj files. That's inconsistent with the > vcxproj files, but it works and Mkvcbuild.pm works as before. > 3) I found why chkpass was needed as a dependency with libpgport and > libpgcommon, actually crypt.c needs to be listed as a unique file, so > it needs a fake entry in vcproj and vcxproj files. > All those things are corrected in the patch attached, tested with both > MS and VC. What's the difference between MS and VC? If VC doesn't accept forward slashes, doesn't that invalidate the premise of this patch? Is it worth attempting to maintain it?
On Mon, Apr 27, 2015 at 9:25 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 4/27/15 2:09 AM, Michael Paquier wrote: >> 2) I noticed that VC builds do not like *at all* file paths with >> forward slashes, perhaps it could be possible to use a Condition like >> hasForwardSlashes but it seems safer to simply enforce the file paths >> to use backslashes in the vcproj files. That's inconsistent with the >> vcxproj files, but it works and Mkvcbuild.pm works as before. >> 3) I found why chkpass was needed as a dependency with libpgport and >> libpgcommon, actually crypt.c needs to be listed as a unique file, so >> it needs a fake entry in vcproj and vcxproj files. >> All those things are corrected in the patch attached, tested with both >> MS and VC. > > What's the difference between MS and VC? MS uses vcxproj specs, and VC vcproj specs... > If VC doesn't accept forward slashes, doesn't that invalidate the > premise of this patch? I don't think so. The point of the patch is to be able to run the Windows build script in a non-Windows environment to check that modifications in the Makefile stucture does not break some basics of the Windows build script, and this is not impacted. > Is it worth attempting to maintain it? vcbuild (~2008) has been replaced by msbuild in MS 2010. Hence I imagine that it is still used a lot. Regards, -- Michael
On 04/27/2015 08:46 AM, Michael Paquier wrote: > On Mon, Apr 27, 2015 at 9:25 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> On 4/27/15 2:09 AM, Michael Paquier wrote: >>> 2) I noticed that VC builds do not like *at all* file paths with >>> forward slashes, perhaps it could be possible to use a Condition like >>> hasForwardSlashes but it seems safer to simply enforce the file paths >>> to use backslashes in the vcproj files. That's inconsistent with the >>> vcxproj files, but it works and Mkvcbuild.pm works as before. >>> 3) I found why chkpass was needed as a dependency with libpgport and >>> libpgcommon, actually crypt.c needs to be listed as a unique file, so >>> it needs a fake entry in vcproj and vcxproj files. >>> All those things are corrected in the patch attached, tested with both >>> MS and VC. >> What's the difference between MS and VC? > MS uses vcxproj specs, and VC vcproj specs... > >> If VC doesn't accept forward slashes, doesn't that invalidate the >> premise of this patch? > I don't think so. The point of the patch is to be able to run the > Windows build script in a non-Windows environment to check that > modifications in the Makefile stucture does not break some basics of > the Windows build script, and this is not impacted. > >> Is it worth attempting to maintain it? > vcbuild (~2008) has been replaced by msbuild in MS 2010. Hence I > imagine that it is still used a lot. > Regards, Yeah. I've pushed this with tiny fixes to avoid Leaning Toothpick Syndrome (man perlop for definition). Thanks for your work on this. cheers andrew
On Mon, Apr 27, 2015 at 10:59 AM, Andrew Dunstan <andrew@dunslane.net> wrote: > Yeah. I've pushed this with tiny fixes to avoid Leaning Toothpick Syndrome > (man perlop for definition). I had to Google that. Then I had to think about it. Then I laughed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company