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

[openjpeg] 16645 Fix compile w/features jp3d,jpip,jpwl,mj2 triplet x64-windows VS 2019 ver 16.9.1 #16661

Merged
merged 10 commits into from
Mar 15, 2021
Merged

Conversation

StarGate-One
Copy link
Contributor

@StarGate-One StarGate-One commented Mar 11, 2021

Describe the pull request

  1. With update to VS 2019 to ver 16.9.1, the openjpeg with any of the features jp3d,jpip,jpwl,mj2 failed to compile under triplet x64-windows.
  2. Created patch fix-lrintf-to-opj_lrintf.patch to change /src/lib/openmj2/opj_includes.h line 96:
    • From: static INLINE long lrintf(float f)
    • To: static INLINE long opj_lrintf(float f)
  3. Modified portfile.cmake to apply patch
  4. Converted CONTROL file to vcpkg.json file
  5. Updated port-version to 3
  6. ran vcpkg x-add-version openjpeg to Update json file
  7. Tested locally on VS 2017/2019 x64-windows triplet
  8. Reformatted vcpkg.json file
  9. Updated port-version to 4

- Modified portfile.cmake to apply patch
  fix-lrintf-to-opj_lrintf.patch
- Added patch to change /src/lib/openmj2/opj_includes.h
  From: static INLINE long lrintf(float f)
  To:   static INLINE long opj_lrintf(float f)
- Converted CONTROL file to vcpkg.json file
- Fix was tested locally on triplet x64-windows with
  Microsoft Windows x64 Professional 20H2 [Version 10.0.19042.867]
    Windows Feature Experience Pack 120.2212.551.0
  Windows SDK [Version 10.0.19041.0]
  VS 2017:
    Visual Studio 2017 Community Edition [Version 15.9.34]
    Visual Studio Toolset [Version 14.16.27023]
    Microsoft (R) C/C++ Optimizing Compiler [Version 19.16.27045 for x64]
  VS 2019:
    Visual Studio 2019 Community Edition [Version 16.9.1]
    Visual Studio Toolset [Version 14.28.29910]
    Microsoft (R) C/C++ Optimizing Compiler [Version 19.28.29912 for x64]
@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label Mar 12, 2021
ports/openjpeg/portfile.cmake Outdated Show resolved Hide resolved
ports/openjpeg/vcpkg.json Outdated Show resolved Hide resolved
versions/baseline.json Outdated Show resolved Hide resolved
versions/o-/openjpeg.json Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

Hi @StarGate-One

Thanks for fixing this issue.

Since upstream suggests vcpkg to only support openjp2. Please see uclouvain/openjpeg#1333 (comment)

So for other features in vcpkg, I'm not sure if it's necessary to add them now.

We have known the way to fix the problem, I think upstream also have known this. But why do they not support other features?

What's your opinion?

StarGate-One and others added 4 commits March 11, 2021 23:12
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@StarGate-One
Copy link
Contributor Author

Hi @NancyLi1013

  1. I agree with your review changes - I clicked on commit suggestion for each one.
  2. So we have the current version fixed.
  3. Also I agree on not adding the new features or removing any unsupported features from upstream into vcpkg until the requirement is needed to upgrade to a newer version.

Thank you for your assistance/help.

  • I will try to do better next time, I got caused myself to get messed up converting the CONTROL file to vcpkg.json file.
  • Even though the format looked correct based on the CONTROL file, it seems vcpkg wants the json file to have features sorted alphabetically and change the windows carriage returns to unix line feeds.

Have a super wonderful weekend!!! 😊✨👏👍

@NancyLi1013
Copy link
Contributor

For the failure on x86-windows:

Found the following errors:
Error: While reading versions for port openjpeg from file: C:\a\1\s\versions\o-\openjpeg.json
       File declares version `2.3.1#3` with scheme: `version-string`.
       But local port declares the same version with a different scheme: `version-semver`.
       Version must be unique even between different schemes.
       Run:

           vcpkg x-add-version openjpeg --overwrite-version

       to overwrite the declared version's scheme.

You can run vcpkg x-add-version openjpeg --overwrite-version and then submit the changes.

@NancyLi1013
Copy link
Contributor

Thanks for your instant response @StarGate-One.

You have done great work now. 😊

Also you can update the format in portfile.cmake if you have enough time to do this work.

Since I cannot review them directly via suggested change. Of course, this is not related with your changes, just for looking more clear or tidy.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Mar 15, 2021
@ras0219-msft ras0219-msft merged commit 5f3249e into microsoft:master Mar 15, 2021
@ras0219-msft
Copy link
Contributor

LGTM, thanks for the fix! I agree that given upstream's position we should probably not offer the extra features.

@StarGate-One StarGate-One deleted the openjpeg-16645 branch March 16, 2021 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[openjpeg] build failure x64-windows with features (jp3d,jpip,jpwl,mj2) on VS 2019 latest update 16.9.1
3 participants