Skip to content

[vcpkg baseline][ideviceinstaller] Fixing error LNK2005 when argtable2 installed before ideviceinstaller#28459

Merged
vicroms merged 27 commits intomicrosoft:masterfrom
Cheney-W:Dev/Cheney/ideviceinstaller
Dec 28, 2022
Merged

[vcpkg baseline][ideviceinstaller] Fixing error LNK2005 when argtable2 installed before ideviceinstaller#28459
vicroms merged 27 commits intomicrosoft:masterfrom
Cheney-W:Dev/Cheney/ideviceinstaller

Conversation

@Cheney-W
Copy link
Copy Markdown
Contributor

@Cheney-W Cheney-W commented Dec 20, 2022

Describe the pull request

  • What does your PR fix?

    ideviceinstaller:x64-windows-static install failed, this error is currently blocking some PRs.
    This issue could be fixed by using getopt as a dependence of argtable2.

    Below is the detailed info of this error:

  Failure:
  getopt.lib(getopt.c.obj) : error LNK2005: optind already defined in argtable2.lib(getopt.c.obj)
  getopt.lib(getopt.c.obj) : error LNK2005: opterr already defined in argtable2.lib(getopt.c.obj)
  getopt.lib(getopt.c.obj) : error LNK2005: optopt already defined in argtable2.lib(getopt.c.obj)

This issue only occurs when argtable2 installed before ideviceinstaller.
getopt is a dependence of ideviceinstaller, argtable2 is not, but getopt and argtable2 have same symbols:

  src\CMakeFiles\argtable2.dir\getopt.c.obj    opterr
  src\CMakeFiles\argtable2.dir\getopt.c.obj    optind
  src\CMakeFiles\argtable2.dir\getopt.c.obj    optopt

  CMakeFiles\getopt.dir\getopt.c.obj    opterr
  CMakeFiles\getopt.dir\getopt.c.obj    optind
  CMakeFiles\getopt.dir\getopt.c.obj    optopt

Since ideviceinstaller installed by vcpkg_install_msbuild() and use USE_VCPKG_INTEGRATION parameter, it will link all of the .lib file under vcpkg/installed\x64-windows-static\lib, then the error LNK2005 occurs.

@Cheney-W Cheney-W added category:port-bug The issue is with a library, which is something the port should already support info:internal labels Dec 20, 2022
github-actions[bot]
github-actions bot previously approved these changes Dec 20, 2022
@Neumann-A
Copy link
Copy Markdown
Contributor

argtable2 is not, but getopt and argtable2 have same symbols:

So argtable is missing a dependency on getopt ?

@Neumann-A
Copy link
Copy Markdown
Contributor

Yeah after checking argtable2.
https://github.com/jonathanmarvens/argtable2/blob/b47cd9b9a13405f4d4656a89bde2c00896c98d82/CMakeLists.txt#L10
is definitely missing a dependency on getopt.

@Neumann-A
Copy link
Copy Markdown
Contributor

Be aware: You probably need to patch the CMake buildsystem to actually respect that setting.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/ideviceinstaller/vcpkg.json b/ports/ideviceinstaller/vcpkg.json
index 8f3f66d..ff9ea70 100644
--- a/ports/ideviceinstaller/vcpkg.json
+++ b/ports/ideviceinstaller/vcpkg.json
@@ -5,7 +5,7 @@
   "description": "Manage apps of iOS devices",
   "homepage": "https://libimobiledevice.org/",
   "license": "LGPL-2.1-only",
-  "supports": "windows & !arm64 ",
+  "supports": "windows & !arm64",
   "dependencies": [
     "libimobiledevice",
     "libzip"

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/argtable2/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions[bot]
github-actions bot previously approved these changes Dec 20, 2022
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/argtable2/vcpkg.json

Valid values for the license field can be found in the documentation

@Cheney-W Cheney-W changed the title [vcpkg baseline][ideviceinstaller] Add support !static [vcpkg baseline][ideviceinstaller] Fixing error LNK2005 when argtable2 installed before ideviceinstaller Dec 20, 2022
github-actions[bot]
github-actions bot previously approved these changes Dec 20, 2022
@Neumann-A
Copy link
Copy Markdown
Contributor

Also the error in ideviceinstaller was due to the msbuild vcpkg integration wasn't it?

@Cheney-W
Copy link
Copy Markdown
Contributor Author

It seems that not only argtable2 himself compiled getopt, gdcm also exists this situation.

github-actions[bot]
github-actions bot previously approved these changes Dec 23, 2022
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider replacing the deprecated license identifiers in the following files:

  • ports/argtable3/vcpkg.json (has deprecated license BSD-2-Clause-FreeBSD)

Deprecated and non deprecated license identifiers can be found here

github-actions[bot]
github-actions bot previously approved these changes Dec 23, 2022
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider replacing the deprecated license identifiers in the following files:

  • ports/argtable3/vcpkg.json (has deprecated license BSD-2-Clause-FreeBSD)

Deprecated and non deprecated license identifiers can be found here

@Cheney-W Cheney-W marked this pull request as ready for review December 23, 2022 14:16
github-actions[bot]
github-actions bot previously approved these changes Dec 23, 2022
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider replacing the deprecated license identifiers in the following files:

  • ports/argtable3/vcpkg.json (has deprecated license BSD-2-Clause-NetBSD)

Deprecated and non deprecated license identifiers can be found here

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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 ideviceinstaller have changed but the version was not updated
version: 1.1.2.23#5
old SHA: 3e54543ef504b74edb836a2c47abb3845b83b7c3
new SHA: 422b75014378e1854bc318130f8ee2165245d6b5
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 vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider replacing the deprecated license identifiers in the following files:

  • ports/argtable3/vcpkg.json (has deprecated license BSD-2-Clause-NetBSD)

Deprecated and non deprecated license identifiers can be found here

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where you should check the license field.

Details

If you feel able to do so, please consider replacing the deprecated license identifiers in the following files:

  • ports/argtable3/vcpkg.json (has deprecated license BSD-2-Clause-NetBSD)

Deprecated and non deprecated license identifiers can be found here

@Cheney-W Cheney-W added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Dec 23, 2022
@vicroms vicroms merged commit 25663f2 into microsoft:master Dec 28, 2022
@reitowo
Copy link
Copy Markdown
Contributor

reitowo commented Dec 31, 2022

Still not fixed in last ci run #28467 @Cheney-W

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Dec 31, 2022

Now it is vendored getopt in darknet.lib..

@Cheney-W
Copy link
Copy Markdown
Contributor Author

Cheney-W commented Jan 4, 2023

Fixing by #28718.

@tomghuang
Copy link
Copy Markdown
Contributor

@Cheney-W , it has been a long time since you added a patch for the argtable3 port, so I'm not sure if you still remember the details. However, I'm curious why you had to patch argtable3 while this issue is about argtable2? Thanks.

@Cheney-W
Copy link
Copy Markdown
Contributor Author

The reason for modifying argtable3 was that it could trigger issues with ideviceinstaller at the time. However, I've now found that even after reverting the previous changes, this port no longer affects ideviceinstaller. I suspect this is due to updates in ideviceinstaller.

@tomghuang
Copy link
Copy Markdown
Contributor

@Cheney-W , appreciate your investigation. If ideviceinstaller still works without Fix-dependence-getopt.patch, then I may remove this patch in order to make the port cleaner. I'll push a PR when the new Argtable3 v3.3.0 is released. Thanks.

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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants