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

[libaec] Fix usage call error #43917

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Conversation

jimwang118
Copy link
Contributor

@jimwang118 jimwang118 commented Feb 19, 2025

Fix the following error.

Cannot open include file: 'libaec.h': No such file or directory 
Error		CMake Error at F:\testspace\tszip\CMakeLists.txt:17 (target_link_libraries):
  Target "tszip" links to:

    libaec::sz

  but the target was not found.  Possible reasons include:

    * There is a typo in the target name.
    * A find_package call is missing for an IMPORTED target.
    * An ALIAS target is missing.
  • 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.

Usage test passed with x64-windows triplet.

@jimwang118 jimwang118 added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. labels Feb 19, 2025
@BillyONeal
Copy link
Member

BillyONeal commented Feb 19, 2025

I think we need a better understanding of the original problem before we start trying to change where this port is installing things. In particular, if someone has made a separate 'szip' port, trying to hide a conflict with it by moving headers and libs elsewhere here doesn't actually fix the problem, as they'll still be left with conflicts at link time.

The right change might be to add a port 'szip' and change libaec to point to it, but I think the customer needs to explain how they got in that situation better.

@jimwang118
Copy link
Contributor Author

I think we need a better understanding of the original problem before we start trying to change where this port is installing things. In particular, if someone has made a separate 'szip' port, trying to hide a conflict with it by moving headers and libs elsewhere here doesn't actually fix the problem, as they'll still be left with conflicts at link time.

The right change might be to add a port 'szip' and change libaec to point to it, but I think the customer needs to explain how they got in that situation better.

OK, I will continue to follow up with the customer to understand the real scenario that caused this issue.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 20, 2025

The original problem is having the now-delisted szip from the past installed while adding libaec. It is a transition problem.

This PR isn't helpful IMO. There could be extra checks and message, but they would not cover all installation orders anyways. There could be an empty szip depending on the new alternative.

@jimwang118 jimwang118 changed the title [libaec] Fix header file conflicts [libaec] Fix usage call error Feb 20, 2025
@jimwang118
Copy link
Contributor Author

The original problem is having the now-delisted szip from the past installed while adding libaec. It is a transition problem.

This PR isn't helpful IMO. There could be extra checks and message, but they would not cover all installation orders anyways. There could be an empty szip depending on the new alternative.

Removed header file conflict fix. Only fix usage errors.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 20, 2025

Please provide a minimum reprocible example of the problem you are trying to fix.
You can do that in in vcpkg-ci-libaec test port.

Once the problem is confirmed, and it is a port problem, we can talk about a fix in the port.

Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

The szip compatible interface is an optional add-on.
You need to test both interfaces independently because they are used independently.

find_package(libaec CONFIG REQUIRED)

add_executable(compression main.c)
target_link_libraries(compression PRIVATE sz aec sz_shared aec_shared)
Copy link
Contributor

Choose a reason for hiding this comment

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

It is obvious that this doesn't make sense.
(Yes, this is what the uninformed heuristics prints. I expect you to see the problems.)

find_package(libaec CONFIG REQUIRED)

add_executable(compression main.c)
target_link_libraries(compression PRIVATE sz aec sz_shared aec_shared)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, maybe don't try to fix a port until you found out how it is meant to be used.
AFAICT proper use generally lists exactly one target (expression) after PRIVATE.

This is probably the most important file in the test.

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:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants