[freeimage] Updated port to prepare for libraw version 0.21 #29605

aklemets · 2023-02-11T22:32:18Z

Updated the freeimage port to prepare it for libraw version 0.21 (which will be added in a separate PR,)

Changes comply with the maintainer guide
SHA512s are updated for each updated download
The "supports" clause reflects platforms that may be fixed by this new version
Any fixed CI baseline entries are removed from that file.
Any patches that are no longer applied are deleted from the port's directory.
The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
Only one version is added to each modified port's versions file.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

aklemets · 2023-02-12T06:19:03Z

@JonLiu1993 If you are able to merge this PR for me, then I can submit a PR for the LibRaw 0.21.1 update next. I don't have write access to merge the PR myself.

dg0yt · 2023-02-12T08:20:54Z

ports/freeimage/rawlib-build-fix.patch

+ 	}
+
+     int read(void *buffer, size_t size, size_t count) { 
+-		if(substream) return substream->read(buffer, size, count);


I don't think this pattern of correction is acceptable. With these changes, we would get a vcpkg state where there is still a protected substream member variable in libraw API, but freeimage wouldn't use it. I cannot verify if this is okay.

What might work as a patch for preparing the transition is: conditionally adding a private substream member LibRaw_freeimage_datastream with the guard you already use in another location:

#if LIBRAW_COMPILE_CHECK_VERSION_NOTLESS(0, 21) LibRaw_abstract_datastream *substream = nullptr; # polyfill #endif

This patch would be much smaller and easier to trust.
And the final patch should also be proposed to upstream.

That's a good suggestion. Thanks! I have pushed an update to the PR.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for freeimage have changed but the version was not updated
version: 3.18.0#25
old SHA: e396e22aa14864a00bf2684cc98503c5c245584a
new SHA: be93b16bc021f1ea376ee6844fe95f09567ccae1
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

aklemets · 2023-02-12T08:55:41Z

I am having a problem with the bot that asks to have the version number updated one more time even though the PR has not been committed yet. This is very strange. I might have to close this PR and open a new one.

dg0yt · 2023-02-12T08:57:46Z

Run vcpkg x-add-version --overwrite-version freeimage, then commit the resulting changes.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

dg0yt · 2023-02-12T12:30:27Z

ports/freeimage/rawlib-build-fix.patch

 	long _eof;
 	INT64 _fsize;

 +#if LIBRAW_COMPILE_CHECK_VERSION_NOTLESS(0, 21)
+protected:
 +	LibRaw_abstract_datastream *substream = nullptr; // polyfill


Could we make it const?

i.e. LibRaw_abstract_datastream * const substream.

It can't be "const" because then the value cannot be modified. It would serve no purpose, and it would be better to delete the variable in that case. (I am going to write a longer explanation in reply to your other comment.)

dg0yt · 2023-02-12T12:31:17Z

ports/freeimage/rawlib-build-fix.patch

+ 	~LibRaw_freeimage_datastream() {
+#if LIBRAW_COMPILE_CHECK_VERSION_NOTLESS(0, 21)
+		if (substream)
+			delete substream;


Is this needed at all? (If substream can be made const, it will always be nullptr.)

But it can't be "const", wasn't that the whole point of your original comment? The way I interpreted your original comment is that you are concerned that someone might define a new class that inherits from the Freeimage datastream class, and then attempt to access the "substream" member that is exposed by the Libraw datastream class. To me, that seems very unlikely, because if they wanted to use the "substream" member, which effectively overrides all the functionality in the Freeimage datastream class, then why not use the Libraw datastream class directly?

However unlikely, your initial concern seemed valid. So, that is why I modified the PR and added a "substream" field to the Freeimage datastream class, to compensate for the field that is going away from the Libraw datastream class.

To make sure that outside users can access the field, it needs the declared "protected", just like the original field was, and it must be non-const. Also, I looked at the sources for the older version of Libraw and they are deleting the "substream" field in the destructor of their class, so to ensure that there will be no memory leak I must do the same thing in the destructor of the Freeimage datastream class.

If you think all this is necessary because you don't think anyone will use "substream", then instead of declaring it as a private variable that is "const" I think my first iteration of this PR is better. Just removing all instances where "substream" is accessed will be the best fix, since it is "dead code". I don't like having useless code around: If nobody will be using "substream" it is better to just remove it rather than defining it as a const null pointer. What do you think?

FTR you are right that the substream member is no longer supported in the next version of freeimage:
https://sourceforge.net/p/freeimage/svn/1903/log/?path=/FreeImage/trunk/Source/FreeImage/PluginRAW.cpp

My point was to ask for a really minimal effective change, scoped to the libraw version which needs this change.
I admit that I didn't consider derived classes in downstream code. Now that I start to think about this case, it seems to me that a private const member would be a good choice anyways: The change will be immediately noticed at build time when downstream derived classes would try to access the member. Downstream could stick to the current version (of libraw!) until they modified their code base.

IMO you could have left this change bundled with the libraw PR. The need for the small patch might be easier to understand with that context. It is normal for vcpkg with its "single consistent universe" approach to versioning.

Disclaimer: I'm just an active contributor, not an official reviewer. Feel free to wait for more feedback before making more changes.

Thanks for pointing me to that source code. I see that while there has not been a new release of FreeImage since 2018, in 2020 they did make a change to make it compile with LibRaw version 0.20. That change is just like what I did in my initial PR. So, actually my initial PR should have been just fine.
That said, I agree with you that having a small patch is nice, so I made an update to the PR where I keep the changes to the minimum and add a comment, explaining what the patch is for.

I cannot add a "const" declaration, however, because it causes a build failure. Because the compiler knows the value is always a null pointer, it results in an error when it tries to dereference the null pointer a few lines below.

If you think this latest iteration looks good can you go ahead and merge this PR for me, as I don't have write access. I want to be able to move on to the LibRaw update.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

jimwang118

You have modified or added at least one portfile where deprecated functions are used.

详情
If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
"name": "vcpkg-cmake",
"host": true
},
{
"name": "vcpkg-cmake-config",
"host": true
}
The following files are affected:

ports/freeimage/portfile.cmake

aklemets · 2023-02-13T06:29:20Z

You have modified or added at least one portfile where deprecated functions are used.

详情 If you feel able to do so, please consider migrating them to the new functions:

Yeah, and I don't care. Freeimage is "abandon-ware". It has had no new official release since 2018. I am just updating the port to avoid a build break that will happen once RawImage is updated to version 0.21.1. I will happily defer cosmetic changes to the portfile.cmake to whoever claims ownership of Freeimage? Are you volunteering?

jimwang118 · 2023-02-14T01:33:27Z

Yeah, and I don't care. Freeimage is "abandon-ware". It has had no new official release since 2018. I am just updating the port to avoid a build break that will happen once RawImage is updated to version 0.21.1. I will happily defer cosmetic changes to the portfile.cmake to whoever claims ownership of Freeimage? Are you volunteering?

You should replace cpkg_configure_cmake with vcpkg_cmake_configure, and github-actions will no longer prompt errors. In the new version of vcpkg, we have optimized and changed some functions, and vcpkg_cmake_configure is one of them.

github-actions

You have modified or added at least one portfile where deprecated functions are used.

If you feel able to do so, please consider migrating them to the new functions:
vcpkg_install_cmake -> vcpkg_cmake_install (from port vcpkg-cmake)
vcpkg_build_cmake -> vcpkg_cmake_build (from port vcpkg-cmake)
vcpkg_configure_cmake -> vcpkg_cmake_configure (Please remove the option PREFER_NINJA) (from port vcpkg-cmake)
vcpkg_fixup_cmake_targets -> vcpkg_cmake_config_fixup (from port vcpkg-cmake-config)
vcpkg_extract_source_archive_ex -> vcpkg_extract_source_archive
vcpkg_build_msbuild -> vcpkg_install_msbuild
vcpkg_copy_tool_dependencies -> vcpkg_copy_tools
vcpkg_apply_patches should be replaced by the PATCHES arguments to the "extract" helpers (e.g. vcpkg_from_github())

In the ports that use the new function, you have to add the corresponding dependencies:

{
  "name": "vcpkg-cmake",
  "host": true
},
{
  "name": "vcpkg-cmake-config",
  "host": true
}

The following files are affected:

ports/freeimage/portfile.cmake

jimwang118 · 2023-02-14T01:58:37Z

ports/freeimage/vcpkg.json

+    {
+      "name": "vcpkg-cmake",
+      "host": true
+    },
    "zlib"


Suggested change

{

"name": "vcpkg-cmake",

"host": true

},

"zlib"

{

"name": "vcpkg-cmake",

"host": true

},

{

"name": "vcpkg-cmake-config",

"host": true

},

"zlib"

dg0yt · 2023-02-14T06:33:45Z

ports/freeimage/rawlib-build-fix.patch

+	// Once the port of FreeImage has been updated to a version greater
+	// than 3.18.0, this patch should be removed as it will not be needed.
+#if LIBRAW_COMPILE_CHECK_VERSION_NOTLESS(0, 20)
+	LibRaw_abstract_datastream const *substream = nullptr;


For the record the const was meant to come after the *: A const pointer, not a pointer to const.

aklemets added 2 commits February 11, 2023 14:27

[freeimage] Updated port to prepare for libraw version 0.21

e3be949

Update baseline

0f4494c

github-actions bot previously approved these changes Feb 11, 2023

View reviewed changes

dg0yt reviewed Feb 12, 2023

View reviewed changes

Simpler build fix

cf6c639

aklemets dismissed github-actions[bot]’s stale review via cf6c639 February 12, 2023 08:44

github-actions bot requested changes Feb 12, 2023

View reviewed changes

Updated the hash of latest freeimage changees

ce9bc08

github-actions bot previously approved these changes Feb 12, 2023

View reviewed changes

aklemets added 2 commits February 12, 2023 01:59

Make it possible to use substream

413d8f3

Updated freeimage port hash

1c8f952

aklemets dismissed github-actions[bot]’s stale review via 1c8f952 February 12, 2023 10:01

github-actions bot previously approved these changes Feb 12, 2023

View reviewed changes

dg0yt reviewed Feb 12, 2023

View reviewed changes

aklemets added 2 commits February 12, 2023 10:42

Add comment and simplify patch

4b5cb4f

Updated freeimage port hash

92d9a5f

aklemets dismissed github-actions[bot]’s stale review via 92d9a5f February 12, 2023 18:43

github-actions bot previously approved these changes Feb 12, 2023

View reviewed changes

aklemets added 2 commits February 12, 2023 10:58

Remove const declaration

2cb660c

Updated freeimage port hash

5493303

aklemets dismissed github-actions[bot]’s stale review via 5493303 February 12, 2023 18:59

github-actions bot previously approved these changes Feb 12, 2023

View reviewed changes

FrankXie05 assigned jimwang118 Feb 13, 2023

jimwang118 added the category:port-update The issue is with a library, which is requesting update new revision label Feb 13, 2023

jimwang118 reviewed Feb 13, 2023

View reviewed changes

jimwang118 added the requires:author-response label Feb 13, 2023

aklemets added 2 commits February 13, 2023 17:47

Someone asked me to make changes to the portfile on their behalf

01fea71

Updated freeimage port hash

0db2c76

aklemets dismissed github-actions[bot]’s stale review via 0db2c76 February 14, 2023 01:48

github-actions bot approved these changes Feb 14, 2023

View reviewed changes

jimwang118 requested changes Feb 14, 2023

View reviewed changes

aklemets closed this Feb 14, 2023

dg0yt reviewed Feb 14, 2023

View reviewed changes

dg0yt mentioned this pull request Feb 14, 2023

[libraw] Update to 0.21.1, revise dependencies #29647

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[freeimage] Updated port to prepare for libraw version 0.21 #29605

[freeimage] Updated port to prepare for libraw version 0.21 #29605

aklemets commented Feb 11, 2023

github-actions bot left a comment

aklemets commented Feb 12, 2023

dg0yt Feb 12, 2023

aklemets Feb 12, 2023

github-actions bot left a comment

aklemets commented Feb 12, 2023

dg0yt commented Feb 12, 2023

github-actions bot left a comment

github-actions bot left a comment

dg0yt Feb 12, 2023

dg0yt Feb 12, 2023

aklemets Feb 12, 2023

dg0yt Feb 12, 2023

aklemets Feb 12, 2023

dg0yt Feb 12, 2023

aklemets Feb 12, 2023

github-actions bot left a comment

github-actions bot left a comment

jimwang118 left a comment

aklemets commented Feb 13, 2023

jimwang118 commented Feb 14, 2023

github-actions bot left a comment

jimwang118 Feb 14, 2023

dg0yt Feb 14, 2023

[freeimage] Updated port to prepare for libraw version 0.21 #29605

[freeimage] Updated port to prepare for libraw version 0.21 #29605

Conversation

aklemets commented Feb 11, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

aklemets commented Feb 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

aklemets commented Feb 12, 2023

dg0yt commented Feb 12, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

jimwang118 left a comment

Choose a reason for hiding this comment

aklemets commented Feb 13, 2023

jimwang118 commented Feb 14, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment