Skip to content

[taglib] Upgrade to 1.12#16266

Closed
uklotzde wants to merge 3 commits intomicrosoft:masterfrom
uklotzde:taglib-1.12
Closed

[taglib] Upgrade to 1.12#16266
uklotzde wants to merge 3 commits intomicrosoft:masterfrom
uklotzde:taglib-1.12

Conversation

@uklotzde
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde commented Feb 17, 2021

  • Replace CONTROL with vcpkg.json
  • Use Git tag as REF
  • Use official version number (without the date suffix)
  • Link official homepage instead of GitHub project page
  • Add license string according to GitHub repo
    LGPL 2.1 or 3.0? taglib/taglib#999
  • Add missing vcpkg_fixup_pkgconfig() in portfile

@uklotzde
[taglib] Upgrade to 1.12
8a4aa55 
- Replace CONTROL with vcpkg.json
- Use Git tag as REF
- Use official version number (without the date suffix)
- Link official homepage instead of GitHub project page
- Add license string according to GitHub repo
  taglib/taglib#999
- Add missing vcpkg_fixup_pkgconfig() in portfile
@uklotzde uklotzde mentioned this pull request Feb 17, 2021
Closed
@ghost
Copy link
Copy Markdown

ghost commented Feb 17, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

Be-ing
Be-ing reviewed Feb 17, 2021
View reviewed changes
@PhoebeHui PhoebeHui assigned PhoebeHui and JackBoosY and unassigned PhoebeHui Feb 18, 2021
@JackBoosY JackBoosY added the category:port-update The issue is with a library, which is requesting update new revision label Feb 18, 2021
@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Feb 18, 2021

When building x64-uwp:

     5>D:\buildtrees\taglib\src\v1.12-8df32e62b7.clean\taglib\mpeg\id3v2\id3v2framefactory.cpp(120,10): error C4996: 'TagLib::ID3v2::FrameFactory::createFrame': was declared deprecated [D:\buildtrees\taglib\x64-uwp-dbg\taglib\tag.vcxproj]
     5>D:\buildtrees\taglib\src\v1.12-8df32e62b7.clean\taglib\mpeg\id3v2\id3v2frame.cpp(590,3): error C4996: 'TagLib::ID3v2::Frame::Header::setData': was declared deprecated [D:\buildtrees\taglib\x64-uwp-dbg\taglib\tag.vcxproj]
         synchronizedlyricsframe.cpp
         textidentificationframe.cpp
         uniquefileidentifierframe.cpp
         unknownframe.cpp
         unsynchronizedlyricsframe.cpp
         urllinkframe.cpp
         chapterframe.cpp
         tableofcontentsframe.cpp
         podcastframe.cpp
         oggfile.cpp
         oggpage.cpp
         oggpageheader.cpp
         xiphcomment.cpp
         vorbisfile.cpp
         vorbisproperties.cpp
         oggflacfile.cpp
         mpcfile.cpp

Maybe the upstream dropped uwp support now?

Please ignore the gsoap regressions.

This was referenced Feb 21, 2021
Closed
Closed
@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Feb 21, 2021

@uklotzde can you look into the build failures?

@uklotzde
Copy link
Copy Markdown
Contributor Author

uklotzde commented Feb 21, 2021

@uklotzde can you look into the build failures?

The implementation of a deprecated function delegates to another deprecated overload. So what shall we do about it? Submit a bug report for TagLib because this causes builds to fail?

@uklotzde
Copy link
Copy Markdown
Contributor Author

uklotzde commented Feb 21, 2021

In my opinion this is a false positive.

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Feb 22, 2021

@uklotzde Can you please report this to taglib upstream?

@JackBoosY JackBoosY added the depends:upstream-changes Waiting on a change to the upstream project label Feb 22, 2021
@uklotzde uklotzde mentioned this pull request Feb 22, 2021
Closed
@uklotzde
Copy link
Copy Markdown
Contributor Author

uklotzde commented Feb 22, 2021 •
edited
Loading

I have submitted taglib/taglib#1000.

Until a fix is available from upstream and for testing the CI builds I have added a temporary patch for CMakeLists.txt that simply disables the C4996 warning.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Feb 23, 2021

Is there anything else to do here? Ready for merge?

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Feb 23, 2021

@Be-ing We need to wait for taglib official reply issue taglib/taglib#1000.

@Be-ing Be-ing mentioned this pull request Feb 25, 2021
Merged
@uklotzde
Copy link
Copy Markdown
Contributor Author

uklotzde commented Mar 10, 2021

Closing this PR after e51bf8e was merged beforehand.

@uklotzde uklotzde closed this Mar 10, 2021
@uklotzde
Copy link
Copy Markdown
Contributor Author

uklotzde commented Mar 10, 2021

Btw, it is discouraging if differing quality criteria are applied for reviewing PRs and well-intended contributions are left behind.

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Mar 10, 2021

@uklotzde Sorry, it seems I overlooked that PR.

JackBoosY
JackBoosY reviewed Mar 10, 2021
View reviewed changes
ports/taglib/msvc-disable-deprecated-warnings.patch
Comment on lines +14 to +17
+ if(ENABLE_STATIC_RUNTIME)
+ foreach(flag_var CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
+ string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}")
+ endforeach(flag_var)
Copy link
Copy Markdown
Contributor

@JackBoosY JackBoosY Mar 10, 2021

Choose a reason for hiding this comment

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

I think this modify is correct, can you please open a new PR to fix that?
See my comment.

Thanks.

@uklotzde uklotzde mentioned this pull request Mar 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@Be-ing Be-ing Be-ing left review comments

@JackBoosY JackBoosY JackBoosY left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@JackBoosY JackBoosY

Labels

category:port-update The issue is with a library, which is requesting update new revision depends:upstream-changes Waiting on a change to the upstream project

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@uklotzde @JackBoosY @Be-ing @PhoebeHui