| Message ID | 20210615134308.236-1-aserdean@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] datapath-windows: Specify external include paths | expand |
On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: > VStudio 16.10 adds usermode includes before including the driver kit ones. > > Bug tracked at: > https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 > > Fixes appveyor build reported by forcing external includes. Thanks, Alin. I know nothing about the windows build process, but I see that this patch fixes the issue with the current AppVeyor CI, therefore: Acked-by: Ilya Maximets <i.maximets@ovn.org> Out of curiosity, is this change backward compatible? I mean, is it possible to build on older platform (older VS) with this change?
On Tue, 2021-06-15 at 18:06 +0200, Ilya Maximets wrote: > On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: > > VStudio 16.10 adds usermode includes before including the driver > > kit ones. > > > > Bug tracked at: > > https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 > > > > Fixes appveyor build reported by forcing external includes. > > Thanks, Alin. I know nothing about the windows build process, but I > see > that this patch fixes the issue with the current AppVeyor CI, > therefore: > > Acked-by: Ilya Maximets <i.maximets@ovn.org> Thank you! > > Out of curiosity, is this change backward compatible? I mean, > is it possible to build on older platform (older VS) with this > change? It should be. Usually we do not need to force the order of include directories. For kernel projects it should default to the kernel includes. I test with the last two versions of VS (2019, 2017). We should add a build matrix for different versions of VS images to appveyor / GHA so we could be sure. I'll try to update the appveyor side. FWIW a new version of VS was launched yesterday ( https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#visual-studio-2019-version-1610-releases ), I will try to compile without the patch to see if they hotfixed the issue.
On Wed, 2021-06-16 at 18:06 +0300, Alin-Gabriel Serdean wrote: > On Tue, 2021-06-15 at 18:06 +0200, Ilya Maximets wrote: > > On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: > > > VStudio 16.10 adds usermode includes before including the driver > > > kit ones. > > > > > > Bug tracked at: > > > https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 > > > > > > Fixes appveyor build reported by forcing external includes. > > > > Thanks, Alin. I know nothing about the windows build process, but > > I > > see > > that this patch fixes the issue with the current AppVeyor CI, > > therefore: > > > > Acked-by: Ilya Maximets <i.maximets@ovn.org> > > Thank you! > > > Out of curiosity, is this change backward compatible? I mean, > > is it possible to build on older platform (older VS) with this > > change? > > It should be. > Usually we do not need to force the order of include directories. For > kernel projects it should default to the kernel includes. > I test with the last two versions of VS (2019, 2017). > > We should add a build matrix for different versions of VS images to > appveyor / GHA so we could be sure. > I'll try to update the appveyor side. > > FWIW a new version of VS was launched yesterday ( > https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#visual-studio-2019-version-1610-releases > ), I will try to compile > without the patch to see if they hotfixed the issue. It did not. Applying the patch.
On 6/17/21 11:31 AM, Alin-Gabriel Serdean wrote: > On Wed, 2021-06-16 at 18:06 +0300, Alin-Gabriel Serdean wrote: >> On Tue, 2021-06-15 at 18:06 +0200, Ilya Maximets wrote: >>> On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: >>>> VStudio 16.10 adds usermode includes before including the driver >>>> kit ones. >>>> >>>> Bug tracked at: >>>> https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 >>>> >>>> Fixes appveyor build reported by forcing external includes. >>> >>> Thanks, Alin. I know nothing about the windows build process, but >>> I >>> see >>> that this patch fixes the issue with the current AppVeyor CI, >>> therefore: >>> >>> Acked-by: Ilya Maximets <i.maximets@ovn.org> >> >> Thank you! >> >>> Out of curiosity, is this change backward compatible? I mean, >>> is it possible to build on older platform (older VS) with this >>> change? >> >> It should be. >> Usually we do not need to force the order of include directories. For >> kernel projects it should default to the kernel includes. >> I test with the last two versions of VS (2019, 2017). >> >> We should add a build matrix for different versions of VS images to >> appveyor / GHA so we could be sure. >> I'll try to update the appveyor side. >> >> FWIW a new version of VS was launched yesterday ( >> https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#visual-studio-2019-version-1610-releases >> ), I will try to compile >> without the patch to see if they hotfixed the issue. > > It did not. Applying the patch. > OK. CI is green now. Thanks! Best regards, Ilya Maximets.
Alin, Does it make sense to apply this patch to stable branches too. I see that 2.15 fails with a similar error. Thanks, Guru On Thu, 17 Jun 2021 at 02:31, Alin-Gabriel Serdean <aserdean@ovn.org> wrote: > On Wed, 2021-06-16 at 18:06 +0300, Alin-Gabriel Serdean wrote: > > On Tue, 2021-06-15 at 18:06 +0200, Ilya Maximets wrote: > > > On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: > > > > VStudio 16.10 adds usermode includes before including the driver > > > > kit ones. > > > > > > > > Bug tracked at: > > > > > https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 > > > > > > > > Fixes appveyor build reported by forcing external includes. > > > > > > Thanks, Alin. I know nothing about the windows build process, but > > > I > > > see > > > that this patch fixes the issue with the current AppVeyor CI, > > > therefore: > > > > > > Acked-by: Ilya Maximets <i.maximets@ovn.org> > > > > Thank you! > > > > > Out of curiosity, is this change backward compatible? I mean, > > > is it possible to build on older platform (older VS) with this > > > change? > > > > It should be. > > Usually we do not need to force the order of include directories. For > > kernel projects it should default to the kernel includes. > > I test with the last two versions of VS (2019, 2017). > > > > We should add a build matrix for different versions of VS images to > > appveyor / GHA so we could be sure. > > I'll try to update the appveyor side. > > > > FWIW a new version of VS was launched yesterday ( > > > https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#visual-studio-2019-version-1610-releases > > ), I will try to compile > > without the patch to see if they hotfixed the issue. > > It did not. Applying the patch. > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Guru, Thank you for letting me know. I backported the change until 2.13. Thank you, Alin. From: Guru Shetty <guru.ovn@gmail.com> Sent: Saturday, June 26, 2021 1:14 AM To: Alin-Gabriel Serdean <aserdean@ovn.org> Cc: Ilya Maximets <i.maximets@ovn.org>; ovs dev <dev@openvswitch.org>; Frank Wagner <frank.wagner@dbosoft.eu> Subject: Re: [ovs-dev] [PATCH] datapath-windows: Specify external include paths Alin, Does it make sense to apply this patch to stable branches too. I see that 2.15 fails with a similar error. Thanks, Guru On Thu, 17 Jun 2021 at 02:31, Alin-Gabriel Serdean <aserdean@ovn.org <mailto:aserdean@ovn.org> > wrote: On Wed, 2021-06-16 at 18:06 +0300, Alin-Gabriel Serdean wrote: > On Tue, 2021-06-15 at 18:06 +0200, Ilya Maximets wrote: > > On 6/15/21 3:43 PM, Alin Gabriel Serdean wrote: > > > VStudio 16.10 adds usermode includes before including the driver > > > kit ones. > > > > > > Bug tracked at: > > > https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 > > > > > > Fixes appveyor build reported by forcing external includes. > > > > Thanks, Alin. I know nothing about the windows build process, but > > I > > see > > that this patch fixes the issue with the current AppVeyor CI, > > therefore: > > > > Acked-by: Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org> > > > Thank you! > > > Out of curiosity, is this change backward compatible? I mean, > > is it possible to build on older platform (older VS) with this > > change? > > It should be. > Usually we do not need to force the order of include directories. For > kernel projects it should default to the kernel includes. > I test with the last two versions of VS (2019, 2017). > > We should add a build matrix for different versions of VS images to > appveyor / GHA so we could be sure. > I'll try to update the appveyor side. > > FWIW a new version of VS was launched yesterday ( > https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#visual-studio-2019-version-1610-releases > ), I will try to compile > without the patch to see if they hotfixed the issue. It did not. Applying the patch.
diff --git a/datapath-windows/ovsext/ovsext.vcxproj b/datapath-windows/ovsext/ovsext.vcxproj index d50a126b4..18f884f41 100644 --- a/datapath-windows/ovsext/ovsext.vcxproj +++ b/datapath-windows/ovsext/ovsext.vcxproj @@ -192,22 +192,39 @@ </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win10 Release|x64'"> <Inf2CatUseLocalTime>true</Inf2CatUseLocalTime> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win10 Debug|x64'"> <Inf2CatUseLocalTime>true</Inf2CatUseLocalTime> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win10Analyze|x64'"> <Inf2CatUseLocalTime>true</Inf2CatUseLocalTime> <CodeAnalysisRuleSet>..\misc\DriverRecommendedRules.ruleset</CodeAnalysisRuleSet> <RunCodeAnalysis>true</RunCodeAnalysis> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8.1Analyze|x64'"> <RunCodeAnalysis>true</RunCodeAnalysis> <CodeAnalysisRuleSet>..\misc\DriverRecommendedRules.ruleset</CodeAnalysisRuleSet> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> </PropertyGroup> <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8Analyze|x64'"> <RunCodeAnalysis>true</RunCodeAnalysis> <CodeAnalysisRuleSet>..\misc\DriverRecommendedRules.ruleset</CodeAnalysisRuleSet> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8 Debug|x64'"> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8 Release|x64'"> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8.1 Debug|x64'"> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> + </PropertyGroup> + <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Win8.1 Release|x64'"> + <ExternalIncludePath>$(CRT_IncludePath);$(KM_IncludePath);</ExternalIncludePath> </PropertyGroup> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Win8 Release|x64'"> <ClCompile>
VStudio 16.10 adds usermode includes before including the driver kit ones. Bug tracked at: https://developercommunity.visualstudio.com/t/error-lnk2019-unresolved-external-symbol-stdio-com/1434674 Fixes appveyor build reported by forcing external includes. Reported-by: Ilya Maximets <i.maximets@ovn.org> Reported-by: Frank Wagner <frank.wagner@dbosoft.eu> Reported-at: https://github.com/openvswitch/ovs-issues/issues/209#issuecomment-861385852 Reported-at: https://github.com/openvswitch/ovs-issues/issues/211 Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org> --- datapath-windows/ovsext/ovsext.vcxproj | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)