New Ticket     Wiki     Browse Source     Timeline     Roadmap     Ticket Reports     Search

Ticket #20525 (closed update: fixed)

Opened 3 years ago

Last modified 3 years ago

testdisk: new version available

Reported by: michael.klein@… Owned by: ryandesign@…
Priority: Normal Milestone:
Component: ports Version: 1.7.1
Keywords: Cc:
Port: testdisk

Description

please update testdisk to 6.11; this version fixes (among other things) the check for uuid_generate() and libuuid in the configure script and works fine with the uuid_* functions provided by libc, so there is no more need to depend on ossp-uuid. Patch for Portfile is attached

TIA

Attachments

testdisk-Portfile.diff Download (1.0 KB) - added by michael.klein@… 3 years ago.

Change History

Changed 3 years ago by michael.klein@…

  Changed 3 years ago by ryandesign@…

  • owner changed from macports-tickets@… to ryandesign@…
  • status changed from new to assigned

follow-up: ↓ 3   Changed 3 years ago by ryandesign@…

  • status changed from assigned to closed
  • resolution set to fixed

Thanks, I committed the update in r54839. I had to add

configure.cppflags
configure.ldflags

otherwise the build failed if ossp-uuid was installed (which it will be for all existing users of testdisk since it was a dependency).

in reply to: ↑ 2 ; follow-up: ↓ 4   Changed 3 years ago by michael.klein@…

{{{ configure.cppflags configure.ldflags }}} otherwise the build failed if ossp-uuid was installed (which it will be for all existing users of testdisk since it was a dependency).

testdisk has quite a few optional dependencies on ports like jpeg or ntfsprogs. With the above fix the configure script won't find the ntfs headers any more even if ntfsprogs is installed. I'd like to suggest to add the following instead:

configure.cppflags-append       [exec sh -c "uuid-config --cflags 2>/dev/null; exit 0"]
configure.ldflags-append        [exec sh -c "uuid-config --ldflags --libs 2>/dev/null; exit 0"]

in reply to: ↑ 3 ; follow-up: ↓ 5   Changed 3 years ago by ryandesign@…

Replying to michael.klein@…:

testdisk has quite a few optional dependencies on ports like jpeg or ntfsprogs. With the above fix the configure script won't find the ntfs headers any more even if ntfsprogs is installed.

A port must not use software it does not declare dependencies on. If we want to have options to use that software, variants should be added that add those dependencies and tell the software it is alright to use that software.

The port needs some dependencies added anyway; see #20528.

I'd like to suggest to add the following instead:

configure.cppflags-append [exec sh -c "uuid-config --cflags 2>/dev/null; exit 0"]
configure.ldflags-append [exec sh -c "uuid-config --ldflags --libs 2>/dev/null; exit 0"]

Well, you can't call a program (like uuid-config) that's provided by another port unless you do it in a phase. We could do that in a pre-configure phase. I simplified your suggestion a bit and tried adding

pre-configure {
    configure.cppflags-append [exec uuid-config --cflags]
    configure.ldflags-append [exec uuid-config --ldflags --libs]
}

to the port and now it compiles again. But in r54839 we just removed ossp-uuid as a dependency, and with the above changes, it now links again with ossp-uuid. Are you saying the dependency on ossp-uuid should be added back in? The other option is to find a way to tell testdisk not to use ossp-uuid even if it is present.

in reply to: ↑ 4   Changed 3 years ago by michael.klein@…

Replying to ryandesign@…:

Replying to michael.klein@…:

testdisk has quite a few optional dependencies on ports like jpeg or ntfsprogs. With the above fix the configure script won't find the ntfs headers any more even if ntfsprogs is installed.

A port must not use software it does not declare dependencies on. If we want to have options to use that software, variants should be added that add those dependencies and tell the software it is alright to use that software.

Consequently, each additional software that is enabled by a variant should explicitly be disabled in the default configuration then, I guess.

$ ./configure --help | grep -- --without-
  --without-PACKAGE       do not use PACKAGE (same as --with-PACKAGE=no)
  --without-ncurses       disabled use of the curses/ncurses/pdcurses/curses
  --without-ext2fs        disabled use of the ext2fs library (default is NO)
  --without-jpeg          disabled use of the jpeg library (default is NO)
  --without-ntfs          disabled use of the ntfs library (default is NO)
  --without-reiserfs      disabled use of the reiserfs library (default is NO)
  --without-ewf           disabled use of the ewf library (default is NO)
  --without-iconv         disabled use of the iconv library (default is NO)

All of the above optional libraries except reiserfs seem to be available in MacPorts. Should testdisk depend on all and let the user disable specific ones in variants or depend only on the common ones (jpeg,iconv,ncurses), and enable the exotic ones in variants?

The port needs some dependencies added anyway; see #20528.

Well, could this be a side effect of configure.cppflags and configure.ldflags in r54839?

{{{ pre-configure { configure.cppflags-append [exec uuid-config --cflags] configure.ldflags-append [exec uuid-config --ldflags --libs] } }}} to the port and now it compiles again. But in r54839 we just removed ossp-uuid as a dependency, and with the above changes, it now links again with ossp-uuid. Are you saying the dependency on ossp-uuid should be added back in?

Not necessarily; My proposal was to make it compile both with and without ossp-uuid installed. ossp-uuid adds nothing to testdisk over the libc provided uuid_* functions AFAICS, so I can't see why there should be a dependency.

testdisk should either use the system's uuid functions or ossp-uuid's. Before r54839, the configure script found uuid_create() in ossp-uuid's libuuid, but the compilation pulled in /usr/include/uuid/uuid.h (because cppflags weren't set for ossp-uuid) which doesn't contain said function, at least on 10.4, so the compilation failed.

If it is a definitive no-no to use a library without a dependency on the providing port it's fine with me to add the dependency back it, at least it's working now ;-) One could still add a without-ossp-uuid variant.

The other option is to find a way to tell testdisk not to use ossp-uuid even if it is present.

Not without changing the configure script, it seems.

Note: See TracTickets for help on using tickets.