On Wed, 11 Nov 2020 at 13:44, Michael Paquier <mich...@paquier.xyz> wrote: > > On Wed, Nov 11, 2020 at 11:01:57AM +1300, David Rowley wrote: > > I'm still working through some small differences in some of the > > .vcxproj files. I've been comparing these by copying *.vcxproj out to > > another directory with patched and unpatched then diffing the > > directory. See attached txt file with those diffs. Here's a summary of > > some of them: > > Thanks. It would be good to not have those diffs if not necessary. > > > 1. There are a few places that libpq gets linked where it previously did > > not. > > It seems to me that your patch is doing the right thing for adminpack > and that its Makefile has no need to include a reference to libpq > source path, no?
Yeah. Likely a separate commit should remove the -I$(libpq_srcdir) from adminpack and old_snapshot > For dblink and postgres_fdw, the duplication comes from PG_CPPFLAGS. > It does not matter much in practice, but it would be nice to not have > unnecessary data in the project files. One thing that could be done > is to make Project.pm more aware of the uniqueness of the elements > included. But, do we really need -I$(libpq_srcdir) there anyway? > From what I can see, we have all the paths in -I we'd actually need > with or without USE_PGXS. I've changed the patch to do that for the includes. I'm now putting the list of include directories in a hash table to get rid of the duplicates. This does shuffle the order of them around a bit. I've done the same for references too. > > 3. LOWER_NODE gets defined in ltree now where it wasn't before. It's > > defined on Linux. Unsure why it wasn't before on Windows. > > Your patch is grabbing the value of PG_CPPFLAGS from ltree's > Makefile, which is fine. We may be able to remove this flag and rely > on pg_tolower() instead in the long run? I am not sure about > FLG_CANLOOKSIGN() though. I didn't look in detail, but it looks like if we define LOWER_NODE on Windows that it might break pg_upgrade. I guess you could say it's partially broken now as the behaviour there will depend on if you build using Visual Studio or cygwin. We'd define LOWER_NODE on cygwin but not on VS. Looks like a pg_upgrade might be problematic there today. It feels a bit annoying to add some special case to the script to maintain the status quo there. An alternative to that would be to modify the .c code at #ifdef LOWER_NODE to also check we're not building on VS. Neither option seems nice. I've attached the updated patch and also a diff showing the changes in the *.vcxproj files. There are quite a few places where the hash table code for includes and references gets rid of duplicates that already exist today. For example pgbench.vcxproj references libpgport.vcxproj and libpgcommon.vcxproj twice. David
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index ebb169e201..2cf3fee65d 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -109,15 +109,13 @@ sub AddDefine
sub WriteReferences
{
my ($self, $f) = @_;
-
- my @references = @{ $self->{references} };
-
- if (scalar(@references))
+ # Add referenced projects, if any exist.
+ if (scalar(keys % { $self->{references} }) > 0)
{
print $f <<EOF;
<ItemGroup>
EOF
- foreach my $ref (@references)
+ foreach my $ref (values % { $self->{references} } )
{
print $f <<EOF;
<ProjectReference Include="$ref->{name}$ref->{filenameExtension}">
@@ -310,11 +308,12 @@ sub WriteItemDefinitionGroup
my $targetmachine =
$self->{platform} eq 'Win32' ? 'MachineX86' : 'MachineX64';
- my $includes = $self->{includes};
- unless ($includes eq '' or $includes =~ /;$/)
+ my $includes = "";
+ foreach my $inc (keys %{ $self->{includes} } )
{
- $includes .= ';';
+ $includes .= $inc . ";";
}
+
print $f <<EOF;
<ItemDefinitionGroup
Condition="'\$(Configuration)|\$(Platform)'=='$cfgname|$self->{platform}'">
<ClCompile>
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index f92c14030d..8db5def9ce 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -32,16 +32,13 @@ my $libpq;
my @unlink_on_exit;
# Set of variables for modules in contrib/ and src/test/modules/
-my $contrib_defines = { 'refint' => 'REFINT_VERBOSE' };
-my @contrib_uselibpq = ('dblink', 'oid2name', 'postgres_fdw', 'vacuumlo');
-my @contrib_uselibpgport = ('oid2name', 'pg_standby', 'vacuumlo');
-my @contrib_uselibpgcommon = ('oid2name', 'pg_standby', 'vacuumlo');
+my $contrib_defines = {};
+my @contrib_uselibpq = ();
+my @contrib_uselibpgport = ('pg_standby');
+my @contrib_uselibpgcommon = ('pg_standby');
my $contrib_extralibs = undef;
-my $contrib_extraincludes = { 'dblink' => ['src/backend'] };
-my $contrib_extrasource = {
- 'cube' => [ 'contrib/cube/cubescan.l', 'contrib/cube/cubeparse.y' ],
- 'seg' => [ 'contrib/seg/segscan.l', 'contrib/seg/segparse.y' ],
-};
+my $contrib_extraincludes = {};
+my $contrib_extrasource = {};
my @contrib_excludes = (
'bool_plperl', 'commit_ts',
'hstore_plperl', 'hstore_plpython',
@@ -925,8 +922,8 @@ sub AddTransformModule
# Add PL dependencies
$p->AddIncludeDir($pl_src);
$p->AddReference($pl_proj);
- $p->AddIncludeDir($pl_proj->{includes});
- foreach my $pl_lib (@{ $pl_proj->{libraries} })
+ $p->AddIncludeDir(join(";", $pl_proj->{includes}));
+ foreach my $pl_lib (keys %{ $pl_proj->{libraries} } )
{
$p->AddLibrary($pl_lib);
}
@@ -935,8 +932,8 @@ sub AddTransformModule
if ($type_proj)
{
$p->AddIncludeDir($type_src);
- $p->AddIncludeDir($type_proj->{includes});
- foreach my $type_lib (@{ $type_proj->{libraries} })
+ $p->AddIncludeDir(join(";", $type_proj->{includes}));
+ foreach my $type_lib (keys %{ $type_proj->{libraries} } )
{
$p->AddLibrary($type_lib);
}
@@ -952,6 +949,7 @@ sub AddContrib
my $subdir = shift;
my $n = shift;
my $mf = Project::read_file("$subdir/$n/Makefile");
+ my @projects;
if ($mf =~ /^MODULE_big\s*=\s*(.*)$/mg)
{
@@ -959,6 +957,7 @@ sub AddContrib
my $proj = $solution->AddProject($dn, 'dll', 'contrib',
"$subdir/$n");
$proj->AddReference($postgres);
AdjustContribProj($proj);
+ push @projects, $proj;
}
elsif ($mf =~ /^MODULES\s*=\s*(.*)$/mg)
{
@@ -970,18 +969,90 @@ sub AddContrib
$proj->AddFile("$subdir/$n/$filename");
$proj->AddReference($postgres);
AdjustContribProj($proj);
+ push @projects, $proj;
}
}
elsif ($mf =~ /^PROGRAM\s*=\s*(.*)$/mg)
{
my $proj = $solution->AddProject($1, 'exe', 'contrib',
"$subdir/$n");
AdjustContribProj($proj);
+ push @projects, $proj;
}
else
{
croak "Could not determine contrib module type for $n\n";
}
+ # Process custom compiler flags
+ if ($mf =~ /^PG_CPPFLAGS\s*=\s*(.*)$/mg || $mf =~
/^override\s*CPPFLAGS\s*(?:[\+\:])?=\s*(.*)$/mg)
+ {
+ foreach my $flag (split /\s+/, $1)
+ {
+ if ($flag =~ /^-D(.*)$/)
+ {
+ foreach my $proj (@projects)
+ {
+ $proj->AddDefine($1);
+ }
+ }
+ elsif ($flag =~ /^-I(.*)$/)
+ {
+ foreach my $proj (@projects)
+ {
+ if ($1 eq '$(libpq_srcdir)')
+ {
+
$proj->AddIncludeDir('src\interfaces\libpq');
+ $proj->AddReference($libpq);
+ }
+ }
+ }
+ }
+ }
+
+ if ($mf =~ /^SHLIB_LINK_INTERNAL\s*=\s*(.*)$/mg)
+ {
+ foreach my $lib (split /\s+/, $1)
+ {
+ if ($lib eq '$(libpq)')
+ {
+ foreach my $proj (@projects)
+ {
+
$proj->AddIncludeDir('src\interfaces\libpq');
+ $proj->AddReference($libpq);
+ }
+ }
+ }
+ }
+
+ if ($mf =~ /^PG_LIBS_INTERNAL\s*=\s*(.*)$/mg)
+ {
+ foreach my $lib (split /\s+/, $1)
+ {
+ if ($lib eq '$(libpq_pgport)')
+ {
+ foreach my $proj (@projects)
+ {
+ $proj->AddReference($libpgport);
+ $proj->AddReference($libpgcommon);
+ }
+ }
+ }
+ }
+
+ foreach my $line (split /\n/, $mf)
+ {
+ if ($line =~ /^[A-Za-z0-9_]*\.o:\s(.*)/)
+ {
+ foreach my $file (split /\s+/, $1)
+ {
+ foreach my $proj (@projects)
+ {
+
$proj->AddFileConditional("$subdir/$n/$file");
+ }
+ }
+ }
+ }
+
# Are there any output data files to build?
GenerateContribSqlFiles($n, $mf);
return;
diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm
index 20f79b382b..ad9e4b6dcc 100644
--- a/src/tools/msvc/Project.pm
+++ b/src/tools/msvc/Project.pm
@@ -24,10 +24,10 @@ sub _new
type => $type,
guid => $^O eq "MSWin32" ? Win32::GuidGen() :
'FAKE',
files => {},
- references => [],
- libraries => [],
+ references => {},
+ libraries => {},
suffixlib => [],
- includes => '',
+ includes => {},
prefixincludes => '',
defines => ';',
solution => $solution,
@@ -42,12 +42,25 @@ sub _new
sub AddFile
{
- my ($self, $filename) = @_;
+ my ($self, $filename, $alwaysAddOriginal) = @_;
+ FindAndAddAdditionalFiles($self, $filename);
+ # Add the original file regardless to the return value of
+ # FindAndAddAdditionalFiles
$self->{files}->{$filename} = 1;
return;
}
+sub AddFileConditional
+{
+ my ($self, $filename) = @_;
+ if (FindAndAddAdditionalFiles($self, $filename) != 1)
+ {
+ $self->{files}->{$filename} = 1;
+ }
+ return;
+}
+
sub AddFiles
{
my $self = shift;
@@ -55,11 +68,39 @@ sub AddFiles
while (my $f = shift)
{
- $self->{files}->{ $dir . "/" . $f } = 1;
+ AddFile($self, $dir . "/" . $f, 1);
}
return;
}
+# Handle makefile rules for when file to be added to the project
+# does not exist. Returns 1 when the original file add should be
+# skipped.
+sub FindAndAddAdditionalFiles
+{
+ my $self = shift;
+ my $fname = shift;
+ $fname =~ /(.*)(\.[^.]+)$/;
+ my $filenoext = $1;
+ my $fileext = $2;
+
+ # For .c files, check if either a .l or .y file of the same name
+ # exists and add that too.
+ if ($fileext eq ".c")
+ {
+ for my $ext (".l", ".y")
+ {
+ my $file = $filenoext . $ext;
+ if (-e $file)
+ {
+ AddFileConditional($self, $file);
+ return 1;
+ }
+ }
+ }
+ return 0;
+}
+
sub ReplaceFile
{
my ($self, $filename, $newname) = @_;
@@ -74,14 +115,14 @@ sub ReplaceFile
if ($file eq $filename)
{
delete $self->{files}{$file};
- $self->{files}{$newname} = 1;
+ AddFile($self, $newname);
return;
}
}
elsif ($file =~ m/($re)/)
{
delete $self->{files}{$file};
- $self->{files}{"$newname/$filename"} = 1;
+ AddFile($self, "$newname/$filename");
return;
}
}
@@ -121,7 +162,8 @@ sub AddReference
while (my $ref = shift)
{
- push @{ $self->{references} }, $ref;
+ my $name = $ref->{name};
+ $self->{references}->{$name} = $ref;
$self->AddLibrary(
"__CFGNAME__/" . $ref->{name} . "/" . $ref->{name} .
".lib");
}
@@ -138,7 +180,7 @@ sub AddLibrary
$lib = '"' . $lib . """;
}
- push @{ $self->{libraries} }, $lib;
+ $self->{libraries}->{$lib} = 1;
if ($dbgsuffix)
{
push @{ $self->{suffixlib} }, $lib;
@@ -148,13 +190,12 @@ sub AddLibrary
sub AddIncludeDir
{
- my ($self, $inc) = @_;
+ my ($self, $incstr) = @_;
- if ($self->{includes} ne '')
+ foreach my $inc (split(/;/, $incstr))
{
- $self->{includes} .= ';';
+ $self->{includes}->{$inc} = 1;
}
- $self->{includes} .= $inc;
return;
}
@@ -256,11 +297,11 @@ sub AddDir
if ($f =~ /^\$\(top_builddir\)\/(.*)/)
{
$f = $1;
- $self->{files}->{$f} = 1;
+ AddFile($self, $f);
}
else
{
- $self->{files}->{"$reldir/$f"} = 1;
+ AddFile($self, "$reldir/$f");
}
}
$mf =~ s{OBJS[^=]*=\s*(.*)$}{}m;
@@ -397,7 +438,7 @@ sub GetAdditionalLinkerDependencies
my ($self, $cfgname, $separator) = @_;
my $libcfg = (uc $cfgname eq "RELEASE") ? "MD" : "MDd";
my $libs = '';
- foreach my $lib (@{ $self->{libraries} })
+ foreach my $lib (keys %{ $self->{libraries} })
{
my $xlib = $lib;
foreach my $slib (@{ $self->{suffixlib} })
differences.bz2
Description: Binary data
