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

Rename heif-convert to heif-dec (planned for v1.18.0) #996

Merged
merged 12 commits into from
Jun 26, 2024

Conversation

farindk
Copy link
Contributor

@farindk farindk commented Oct 17, 2023

Renames the executable heif-convert to heif-dec and create a symlink from heif-convert to heif-dec (#985).

Could anyone working on Windows and macOS please verify that this works?
Especially generating the symlink with the correct directories and executable suffix (100d172)?

@kmilos
Copy link
Contributor

kmilos commented Oct 17, 2023

Could anyone working on Windows and macOS please verify that this works?

Does not seem to work under MSYS2:

CMake Error: failed to create symbolic link 'C:/msys64/opt/libheif/bin/heif-convert.exe': The system
 cannot find the path specified.

I reckon you need to move this inside the examples list, and just after heif-dec is actually installed.

@farindk
Copy link
Contributor Author

farindk commented Oct 17, 2023

Makes sense. Better with the above change?

@kmilos
Copy link
Contributor

kmilos commented Oct 17, 2023

Better with the above change?

Better in a sense that it now finds it, but still fails, now with:

CMake Error: failed to create symbolic link 'C:/msys64/opt/libheif/bin/heif-convert.exe': A required
 privilege is not held by the client.

@farindk
Copy link
Contributor Author

farindk commented Oct 17, 2023

A web search told me that Windows requires administrator privileges to create symlinks. (The reason is that symlinks were only introduced with Windows Vista and since it would open security risks when using older software, creating symlinks has been restricted to administrators.)

Now, what should we do for Windows? Copy the binary instead?

@kmilos
Copy link
Contributor

kmilos commented Oct 17, 2023

Now, what should we do for Windows? Copy the binary instead?

Traditionally, yes. I though the CMake create_symlink command was actually behaving this way on Windows, and not trying to make a real symlink...

@silverbacknet
Copy link

Another option some software uses, since static (no plugin) builds can be pretty large, is a stub that simply forwards the entire command environment to the real exe, and returns whatever it does.

With plug-in being the default, that might be overkill for one scenario.

@farindk
Copy link
Contributor Author

farindk commented Oct 17, 2023

@silverbacknet Right, good idea. But in our case, the binary is small (~56 kB). The libheif dll is always separate. It is only the heif-convert tool that would have to be copied.

@farindk farindk changed the title Rename heif-convert to heif-dec Rename heif-convert to heif-dec (planned for v1.18.0) Oct 17, 2023
@fancycode
Copy link
Member

@farindk please make sure to change the manpage to reference the new command name (currently still mentions heif-convert).

CMakeLists.txt Outdated
@@ -260,6 +260,7 @@ configure_file(libheif.pc.in ${CMAKE_CURRENT_BINARY_DIR}/libheif.pc @ONLY)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/libheif.pc
DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig)


Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove non-change from diff

examples/CMakeLists.txt Outdated Show resolved Hide resolved
@farindk farindk changed the base branch from master to develop-v1.18.0 June 11, 2024 10:17
@farindk farindk changed the base branch from develop-v1.18.0 to master June 11, 2024 10:18
@farindk farindk merged commit eb795b8 into master Jun 26, 2024
30 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants