[cppunit] add linux support and bump version to 1.15.1 #15018

dweckmann · 2020-12-09T06:34:42Z

Describe the pull request

What does your PR fix?

Add full linux support and update version to 1.15.1. FindPackage(CppUnit ...) support is also added. We provide a custom CMakelists.txt for windows as it is simpler. Since I'm not a Pro MSVC user, someone should check that it is still usable on this IDE.

Which triplets are supported/not supported? Have you updated the CI baseline?

All triplets should be supported, with the exception of uwp

BillyONeal · 2020-12-09T19:51:44Z

I'm 99% sure this is correct but I'm still not totally up to speed myself on this exception to vcpkg_cmake_wrapper; should get this merged tomorrow after some discussion. Thanks for your contribution!

NancyLi1013

Could you please help confirm if this port supports arm, uwp and osx?

If it can support all triplets now, please remove these codes in ci.baseline.txt.

 cppunit:arm64-windows=fail
 cppunit:arm-uwp=fail
 cppunit:x64-linux=fail
 cppunit:x64-osx=fail
 cppunit:x64-uwp=fail

ports/cppunit/portfile.cmake

NancyLi1013 · 2020-12-10T06:52:02Z

The failures on linux and osx like this:

/mnt/vcpkg-ci/buildtrees/cppunit/src/cppunit-1-60beb2c90e.clean/include/cppunit/Portability.h:18:14: fatal error: cppunit/config-auto.h: No such file or directory
 #    include <cppunit/config-auto.h>
              ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

/mnt/vcpkg-ci/buildtrees/cppunit/src/cppunit-1-60beb2c90e.clean/src/cppunit/DllMain.cpp:8:10: fatal error: windows.h: No such file or directory
 #include <windows.h>
          ^~~~~~~~~~~
compilation terminated.

The failures on uwp like this:

5>D:\buildtrees\cppunit\src\cppunit-1-60beb2c90e.clean\src\cppunit\Win32DynamicLibraryManager.cpp(24,12): error C2039: 'LoadLibraryA': is not a member of '`global namespace'' [D:\buildtrees\cppunit\x64-uwp-dbg\cppunit.vcxproj]
     5>D:\buildtrees\cppunit\src\cppunit-1-60beb2c90e.clean\src\cppunit\Win32DynamicLibraryManager.cpp(24,24): error C3861: 'LoadLibraryA': identifier not found [D:\buildtrees\cppunit\x64-uwp-dbg\cppunit.vcxproj]
     5>C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\XamlCompiler\Microsoft.Windows.UI.Xaml.Common.targets(486,5): error MSB4181: The "CompileXaml" task returned false but did not log an error.

For uwp, I think this port currently doesn't support uwp, we can add Supports: !uwp to vcpkg.json file and also add unsupported message vcpkg_fail_port_install(ON_TARGET "uwp") to portfile.cmake.

For linux and osx, you should try to look into and fix the problems.

dweckmann · 2020-12-10T07:06:30Z

yep, I will look into it. I was too confident :)

dweckmann · 2020-12-10T07:58:14Z

Unfortunately, the unified provided CMakeLists.txt is not doable without patching the upstream source (the thing I've forgotten to commit). I've changed my mind and use now the upstream configure script. This is less dangerous, but leads to maintain two build system.

If you prefer, I can switch back to msbuild for Windows, as there is no added value now to use CMakefile.txt ?

BillyONeal · 2020-12-10T18:10:36Z

If you prefer, I can switch back to msbuild for Windows, as there is no added value now to use CMakefile.txt ?

We have strong opinions about how ports should interact with the rest of the system, such as how they use features, and what ports to do upstream contents (e.g. we aren't everything's bug clearinghouse when upstream hasn't accepted those changes) but beyond that what the contributor says goes :)

dweckmann · 2020-12-10T18:51:27Z

but beyond that what the contributor says goes :)

ok, fine then :)

Frankly speaking, I dislike like hell patches. They are screwed to a specific version and are hard to maintain. So, If everybody agree, I would stick to that proposal, but I'm always open for discussion (and for improvement ;)

BillyONeal · 2020-12-11T00:15:45Z

After some discussion we don't believe this vcpkg_cmake_wrapper is correct here: that's for overriding built-in cmake find scripts only, and we should be emitting a config instead.

dweckmann · 2020-12-11T13:33:25Z

After some discussion we don't believe this vcpkg_cmake_wrapper is correct here: that's for overriding built-in cmake find scripts only, and we should be emitting a config instead.

I've made the required changes. It should be good now.

scripts/ci.baseline.txt

JackBoosY · 2020-12-22T02:18:45Z

After internal discussion, we think we can accept the new cmake build method.

JackBoosY

Can you change the non-windows build method to cmake?

dweckmann · 2020-12-22T09:10:01Z

Can you change the non-windows build method to cmake?

Not easily. It requires patching cppunit, I prefer not too.

JackBoosY · 2020-12-22T09:43:59Z

Fine, that's acceptable.

JackBoosY · 2020-12-22T09:53:53Z

The regression will be fixed in #15251.

BillyONeal · 2020-12-21T22:00:48Z

ports/cppunit/CMakeLists.txt

+
+file(GLOB CPPUNIT_SRC RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}"
+    "${CMAKE_CURRENT_SOURCE_DIR}/src/cppunit/*.cpp")
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/include)


Should this target_include_directories so that folks who link with cppunit have the right headers automatically available instead of needing to author your own Config?

Yes you are right. I will change that ASAP, but as on linux, the build use the old configure script, we still have to deliver our own Config

BillyONeal · 2020-12-21T22:01:33Z

ports/cppunit/CMakeLists.txt

+    ${CMAKE_CURRENT_SOURCE_DIR}/src/DllPlugInTester/DllPlugInTester.cpp
+)
+
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/src/DllPlugInTester)


Ditto target_include_directories.

BillyONeal · 2020-12-22T23:46:46Z

Please merge with master to get infrastructure updates; you created this PR without permission for us to do that for you

dweckmann · 2020-12-23T11:56:28Z

Please merge with master to get infrastructure updates; you created this PR without permission for us to do that for you

Ok, I will do that ASAP, Sorry, I'm not at work right now because of christmas holiday.

BillyONeal · 2020-12-28T23:41:43Z

@NancyLi1013 Can you mark this approved if you're not still waiting for changes?

Thanks!

NancyLi1013 · 2020-12-29T02:28:58Z

LGTM now. If there are no other concerns from you, you can merge this PR @BillyONeal.

BillyONeal · 2020-12-29T03:35:14Z

Woooooo :D

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch 4 times, most recently from 48e1033 to 5d4a8f8 Compare December 9, 2020 07:47

JonLiu1993 assigned NancyLi1013 Dec 9, 2020

BillyONeal closed this Dec 9, 2020

BillyONeal reopened this Dec 9, 2020

BillyONeal added the requires:discussion label Dec 9, 2020

NancyLi1013 added category:port-update The issue is with a library, which is requesting update new revision category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist labels Dec 10, 2020

NancyLi1013 reviewed Dec 10, 2020

View reviewed changes

ports/cppunit/portfile.cmake Outdated Show resolved Hide resolved

NancyLi1013 added the requires:author-response label Dec 10, 2020

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch 2 times, most recently from b30739b to 413a2e5 Compare December 10, 2020 05:54

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch 2 times, most recently from 3cb2111 to 397a3ec Compare December 10, 2020 07:56

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch from 397a3ec to e6d7a24 Compare December 10, 2020 16:44

BillyONeal removed the requires:discussion label Dec 10, 2020

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch from e6d7a24 to 6d605aa Compare December 11, 2020 13:31

NancyLi1013 suggested changes Dec 14, 2020

View reviewed changes

scripts/ci.baseline.txt Outdated Show resolved Hide resolved

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch 2 times, most recently from b4fe144 to abf6986 Compare December 14, 2020 08:11

JackBoosY removed the requires:discussion label Dec 22, 2020

JackBoosY suggested changes Dec 22, 2020

View reviewed changes

JackBoosY added the requires:author-response label Dec 22, 2020

JackBoosY removed the requires:author-response label Dec 22, 2020

JackBoosY approved these changes Dec 22, 2020

View reviewed changes

JackBoosY mentioned this pull request Dec 22, 2020

[vcpkg baseline] Fix uvatlas download file hash #15251

Closed

JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Dec 22, 2020

BillyONeal reviewed Dec 22, 2020

View reviewed changes

NancyLi1013 added the requires:author-response label Dec 23, 2020

JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Dec 23, 2020

dweckmann added 2 commits December 23, 2020 13:02

[cppunit] add linux support and bump version to 1.15.1

72624f9

[cppunit] remove msbuild mention

9b5f029

dweckmann force-pushed the cppunit-linux-and-update-to-1.15.1 branch from 11c68c2 to 9b5f029 Compare December 23, 2020 12:02

JackBoosY requested a review from BillyONeal December 24, 2020 02:40

JackBoosY removed the requires:author-response label Dec 24, 2020

NancyLi1013 approved these changes Dec 29, 2020

View reviewed changes

NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Dec 29, 2020

BillyONeal merged commit e2d3408 into microsoft:master Dec 29, 2020

ryukw7 pushed a commit to ryukw7/vcpkg that referenced this pull request Dec 29, 2020

[cppunit] add linux support and bump version to 1.15.1 (microsoft#15018)

7d19e5f

dweckmann deleted the cppunit-linux-and-update-to-1.15.1 branch January 4, 2021 06:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cppunit] add linux support and bump version to 1.15.1 #15018

[cppunit] add linux support and bump version to 1.15.1 #15018

dweckmann commented Dec 9, 2020 •

edited

Loading

BillyONeal commented Dec 9, 2020

NancyLi1013 left a comment

NancyLi1013 commented Dec 10, 2020

dweckmann commented Dec 10, 2020

dweckmann commented Dec 10, 2020

BillyONeal commented Dec 10, 2020

dweckmann commented Dec 10, 2020

BillyONeal commented Dec 11, 2020

dweckmann commented Dec 11, 2020

JackBoosY commented Dec 22, 2020

JackBoosY left a comment

dweckmann commented Dec 22, 2020

JackBoosY commented Dec 22, 2020

JackBoosY commented Dec 22, 2020

BillyONeal Dec 21, 2020

dweckmann Dec 23, 2020

BillyONeal Dec 21, 2020

BillyONeal commented Dec 22, 2020 •

edited

Loading

dweckmann commented Dec 23, 2020

BillyONeal commented Dec 28, 2020

NancyLi1013 commented Dec 29, 2020

BillyONeal commented Dec 29, 2020

[cppunit] add linux support and bump version to 1.15.1 #15018

[cppunit] add linux support and bump version to 1.15.1 #15018

Conversation

dweckmann commented Dec 9, 2020 • edited Loading

BillyONeal commented Dec 9, 2020

NancyLi1013 left a comment

Choose a reason for hiding this comment

NancyLi1013 commented Dec 10, 2020

dweckmann commented Dec 10, 2020

dweckmann commented Dec 10, 2020

BillyONeal commented Dec 10, 2020

dweckmann commented Dec 10, 2020

BillyONeal commented Dec 11, 2020

dweckmann commented Dec 11, 2020

JackBoosY commented Dec 22, 2020

JackBoosY left a comment

Choose a reason for hiding this comment

dweckmann commented Dec 22, 2020

JackBoosY commented Dec 22, 2020

JackBoosY commented Dec 22, 2020

BillyONeal Dec 21, 2020

Choose a reason for hiding this comment

dweckmann Dec 23, 2020

Choose a reason for hiding this comment

BillyONeal Dec 21, 2020

Choose a reason for hiding this comment

BillyONeal commented Dec 22, 2020 • edited Loading

dweckmann commented Dec 23, 2020

BillyONeal commented Dec 28, 2020

NancyLi1013 commented Dec 29, 2020

BillyONeal commented Dec 29, 2020

dweckmann commented Dec 9, 2020 •

edited

Loading

BillyONeal commented Dec 22, 2020 •

edited

Loading