Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pcre2] Update to 10.35 #11754

Merged

Conversation

c72578
Copy link
Contributor

@c72578 c72578 commented Jun 3, 2020

Describe the pull request

  • What does your PR fix? Fixes #
    • New upstream version 10.35
    • Update patches:
      pcre2-10.35_fix-space.patch
          Reported and accepted upstream: https://bugs.exim.org/show_bug.cgi?id=2588
      pcre2-10.35_fix-uwp.patch
    • Add patch:
      pcre2-10.35_fix_postfix_for_debug_Windows_builds.patch
          Reported and accepted upstream: https://bugs.exim.org/show_bug.cgi?id=2600
    • Drop patch (fixed upstream):
      fix-arm64-config.patch
    • Since 10.35, pkgconfig files are created using CMake
      • fix debug suffix of pkgconfig files under Windows
      • add vcpkg_fixup_pkgconfig()
    • Remove bin and debug/bin in case of static build
    • Update ci.baseline.txt
      Remove "pcre2:arm-uwp=fail", because arm-uwp passes now
      Add "unicorn-lib:arm-uwp=fail"
      Changelog: https://www.pcre.org/changelog.txt
  • Which triplets are supported/not supported? Have you updated the CI baseline?
    All triplets are supported.
    • CI baseline has been updated:
    • Remove "pcre2:arm-uwp=fail", because arm-uwp passes now
    • Add "unicorn-lib:arm-uwp=fail"
      This is required now that pcre2:arm-uwp passes
  • Does your PR follow the maintainer guide?
    Yes

@c72578 c72578 changed the title [pcre2] Update to 10.35 WIP: [pcre2] Update to 10.35 Jun 4, 2020
@c72578
Copy link
Contributor Author

c72578 commented Jun 4, 2020

The following patches have been investigated and updated:

  • fix-space.patch
    This patch is still needed. The patch was updated, so that it applies to the lines of code in PCRE2 10.35.
    Submitted upstream patch: https://bugs.exim.org/show_bug.cgi?id=2588
  • fix-uwp.patch
    A part of this patch is still needed concerning CMakeLists.txt:
    add_compile_options(/wd4146)
    The rest of the patch has been fixed upstream and removed from the patch here.

The following patch is not needed any more, it has been fixed upstream:

  • fix-arm64-config.patch
    src/sljit/sljitConfigInternal.h
    The following has been added upstream and is in PCRE2 v10.35:
#elif defined(_M_ARM64) || defined(__aarch64__)
#define SLJIT_CONFIG_ARM_64 1
...
#elif defined _WIN32

#define SLJIT_CACHE_FLUSH(from, to) \
	FlushInstructionCache(GetCurrentProcess(), (char*)(from), (char*)(to) - (char*)(from))

#else

This is the only missing line, which was in the former fix-arm64-config.patch and is not found in sljitConfigInternal.h of PCRE2 10.35:

#pragma comment(lib, "kernel32.lib")

The CI build of arm64_windows passes, so fix-arm64-config.patch could be removed.

@c72578 c72578 force-pushed the 2020-06-03_pcre2_Update_to_10.35 branch 3 times, most recently from 059953f to 79825c7 Compare June 5, 2020 08:55
@c72578 c72578 changed the title WIP: [pcre2] Update to 10.35 [pcre2] Update to 10.35 Jun 5, 2020
@c72578 c72578 marked this pull request as ready for review June 5, 2020 09:18
@c72578
Copy link
Contributor Author

c72578 commented Jun 5, 2020

@JackBoosY Do you know, why the Azure Pipelines report a failure in case of x64-linux?
There is no error in the build logs, just the following warning in config-x64-linux-dbg-err.log and config-x64-linux-rel-err.log:

CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_INSTALL_BINDIR
    CMAKE_INSTALL_LIBDIR
    VCPKG_PLATFORM_TOOLSET
    VCPKG_SET_CHARSET_FLAG

@JackBoosY
Copy link
Contributor

@c72578 That's not related to this PR, I will restart the linux test later.

@c72578 c72578 force-pushed the 2020-06-03_pcre2_Update_to_10.35 branch 3 times, most recently from 53ef216 to d3f530b Compare June 6, 2020 06:53
@c72578 c72578 requested a review from JackBoosY June 6, 2020 08:04
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unicorn-lib:arm-uwp=fail to VCPKG_PATH/scripts/ci.baseline.txt

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-update The issue is with a library, which is requesting update new revision requires:author-response labels Jun 11, 2020
@c72578
Copy link
Contributor Author

c72578 commented Jun 12, 2020

Please add unicorn-lib:arm-uwp=fail to VCPKG_PATH/scripts/ci.baseline.txt

@JackBoosY do you know, why arm-uwp of unicorn-lib is failing now?
The errors look like this:

4>C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\um\combaseapi.h(1123,10): error C3861: 'CoCreateInstanceFromApp': identifier not found (compiling source file E:\buildtrees\unicorn-lib\src\8e5ec8b92a-e3bfa5feb4\unicorn\path.cpp) [E:\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.vcxproj]
4>C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\um\combaseapi.h(1123,10): error C3861: 'CoCreateInstanceFromApp': identifier not found (compiling source file E:\buildtrees\unicorn-lib\src\8e5ec8b92a-e3bfa5feb4\unicorn\mbcs.cpp) [E:\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.vcxproj]
4>C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\um\combaseapi.h(1144,12): error C3861: 'CoCreateInstanceFromApp': identifier not found (compiling source file E:\buildtrees\unicorn-lib\src\8e5ec8b92a-e3bfa5feb4\unicorn\path.cpp) [E:\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.vcxproj]
4>C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\um\combaseapi.h(1144,12): error C3861: 'CoCreateInstanceFromApp': identifier not found (compiling source file E:\buildtrees\unicorn-lib\src\8e5ec8b92a-e3bfa5feb4\unicorn\mbcs.cpp) [E:\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.vcxproj]
...

Full log from CI attached.
arm-uwp_port_build_failure_logs_2020-06-11_d87047b.zip
This is the artifact from: https://dev.azure.com/vcpkg/public/_build/results?buildId=38626&view=artifacts&type=publishedArtifacts

@JackBoosY
Copy link
Contributor

According to msdn documentation, when using CoCreateInstanceFromApp, we should include combaseapi.h and add Combase.lib to the link list first.

You can check whether the function header file is included by opening VCPKG_PATH\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.sln, and see if the function is undeclared.

@c72578
Copy link
Contributor Author

c72578 commented Jun 13, 2020

@JackBoosY, have you got access to VCPKG_PATH\buildtrees\unicorn-lib\arm-uwp-dbg\unicorn-lib.sln on the Azure pipelines?

@JackBoosY
Copy link
Contributor

@c72578 I think we should deal with unicorn-lib in other PR, in this PR we just need to change it to fail.

@c72578
Copy link
Contributor Author

c72578 commented Jun 15, 2020

@JackBoosY, I have recently updated unicorn-lib, to check, if it builds OK on arm_uwp (before updating PCRE2 here):
#11830
And yes, arm_uwp was OK. The failure of unicorn-lib has appeared here, in the PR for the update of PCRE2.

@JackBoosY
Copy link
Contributor

It fails on SOURCE\unicorn\path.cpp line489:

error C3861: 'CreateFileW': identifier not found

According to msdn documentation, this function only support desktop app, and there is no code to adapt to uwp. So I think that's expected:

    Path::id_type Path::id(flag_type flags) const noexcept {
        uint64_t device = 0, file = 0;
        #ifdef _XOPEN_SOURCE
            auto st = get_stat(filename, flags).st;
            device = uint64_t(st.st_dev);
            file = uint64_t(st.st_ino);
        #else
            (void)flags;
            auto handle = CreateFileW(c_name(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS , nullptr);
            if (handle != INVALID_HANDLE_VALUE) {
                BY_HANDLE_FILE_INFORMATION info;
                memset(&info, 0, sizeof(info));
                if (GetFileInformationByHandle(handle, &info)) {
                    device = uint64_t(info.dwVolumeSerialNumber);
                    file = (uint64_t(info.nFileIndexHigh) << 32) + uint64_t(info.nFileIndexLow);
                }
                CloseHandle(handle);
            }
        #endif
        return {device, file};
    }

@c72578
Copy link
Contributor Author

c72578 commented Jun 16, 2020

@JackBoosY, thanks for the clarification.
I have added unicorn-lib:arm-uwp=fail to VCPKG_PATH/scripts/ci.baseline.txt now.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jun 16, 2020
@c72578
Copy link
Contributor Author

c72578 commented Jun 27, 2020

In the meantime, the pcre2 port has been modified in master:
[vcpkg] Add vcpkg_from_sourceforge (1/2)
a0e0c57
I have updated the PR here to fix the conflicts.

@c72578 c72578 force-pushed the 2020-06-03_pcre2_Update_to_10.35 branch from 66ddbec to cfa3bda Compare June 27, 2020 06:18
@c72578
Copy link
Contributor Author

c72578 commented Jun 27, 2020

Info concerning REGRESSION: paraview:x64-linux, which is unrelated to this PR here:
From install-x64-linux-dbg-err.log:

ninja: error: '/mnt/vcpkg-ci/installed/x64-linux/lib/libmath.a', needed by 'bin/pvserver', missing and no known rule to make it

@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Jun 28, 2020
@JackBoosY
Copy link
Contributor

the paraview regression will be solve in other PR.

@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 28, 2020
@c72578
Copy link
Contributor Author

c72578 commented Jun 29, 2020

Remark: PR #12144 to fix paraview:x64-linux build has been submitted by @JackBoosY

@JackBoosY JackBoosY removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 30, 2020
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jun 30, 2020
@c72578 c72578 force-pushed the 2020-06-03_pcre2_Update_to_10.35 branch 2 times, most recently from 7bbc627 to e8c4994 Compare July 3, 2020 15:10
@c72578
Copy link
Contributor Author

c72578 commented Jul 3, 2020

PR has been rebased due to a recent modification to pcre2 in master:

[pcre2] Restore the https://ftp.pcre.org/ mirror in addition to the SourceForge mirrors. (#12233)
Commit: 3d2a7ca

- New upstream version 10.35
- Update patches:
  pcre2-10.35_fix-space.patch
  pcre2-10.35_fix-uwp.patch
- Add patch:
  pcre2-10.35_fix_postfix_for_debug_Windows_builds.patch
- Drop patch (fixed upstream):
  fix-arm64-config.patch
- Since 10.35, pkgconfig files are created using CMake
  * fix debug suffix of pkgconfig files under Windows
  * add vcpkg_fixup_pkgconfig()
- Remove bin and debug/bin in case of static build
- Update ci.baseline.txt
  Remove "pcre2:arm-uwp=fail", because arm-uwp passes now
  Add "unicorn-lib:arm-uwp=fail"
@c72578 c72578 force-pushed the 2020-06-03_pcre2_Update_to_10.35 branch from e8c4994 to 5f1f6a9 Compare July 3, 2020 15:21
@ras0219-msft
Copy link
Contributor

LGreatTM :)

@ras0219-msft ras0219-msft merged commit 3e615cd into microsoft:master Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants