Opened 12 years ago

Closed 6 years ago

#31667 closed defect (fixed)

libpar2 and par2 fail 'make check' with clang++

Reported by: dinge345@… Owned by: jeremyhu (Jeremy Huddleston Sequoia)
Priority: Normal Milestone:
Component: ports Version: 2.0.3
Keywords: Cc: fracai, julian@…, ryandesign (Ryan Carsten Schmidt), anddam (Andrea D'Amore)
Port: libpar2 par2

Description

libpar2 does not compile:

:info:build ./par2fileformat.h:87:25: error: flexible array member 'entries' of non-POD element type 'FILEVERIFICATIONENTRY []'

but forcing compiler to g++ works:

port install libpar2 configure.compiler=gcc

Using OS X 10.7.2 with XCode 4.2, no older XCode installed. MacPorts defaults to clang in this configuration.

Attachments (2)

main.log (29.2 KB) - added by dinge345@… 12 years ago.
/opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_archivers_libpar2/libpar2/main.log
patch-port_libpar2.diff (1.1 KB) - added by anddam (Andrea D'Amore) 12 years ago.

Download all attachments as: .zip

Change History (24)

Changed 12 years ago by dinge345@…

Attachment: main.log added

/opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_archivers_libpar2/libpar2/main.log

comment:1 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Owner: changed from macports-tickets@… to arno+macports@…

I don't know what "configure.compiler=gcc" means, since Xcode 4.2 does not include any version of gcc; please try with "configure.compiler=llvm-gcc-4.2" instead; if that works, we want to use that.

comment:2 Changed 12 years ago by dinge345@…

Yes, it works with "configure.compiler=llvm-gcc-4.2".

comment:3 Changed 12 years ago by dinge345@…

Unsurprisingly I get the same error with the par2 port, it also works with llvm-gcc-4.2

comment:4 in reply to:  2 Changed 12 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added
Resolution: fixed
Status: newclosed

Replying to dinge345@…:

Yes, it works with "configure.compiler=llvm-gcc-4.2".

Thanks; fixed in r86473.

Replying to dinge345@…:

Unsurprisingly I get the same error with the par2 port

#31789

comment:5 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Resolution: fixed
Status: closedreopened

Reopening to track a real fix rather than a workaround

comment:6 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Owner: changed from arno+macports@… to jeremyhu@…
Status: reopenednew

comment:7 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

This is essentially the same as #33152

comment:8 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Owner: changed from jeremyhu@… to and.damore@…

Andrea,

You found the solution for the similar issue in ppl, so would you mind fixing par2 and libpar2 as well? I'm fluent in C++, and it doesn't look like this is the same root cause (issues in configure).

If this is a deficiency in clang++, we should file a report against the compiler rather than working around it.

comment:9 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Cc: jeremyhu@… added

Cc Me!

comment:10 Changed 12 years ago by anddam (Andrea D'Amore)

Cc: and.damore@… added; jeremyhu@… removed
Owner: changed from and.damore@… to jeremyhu@…

The error refers to par2fileformat.h:87 but that doesn't seems a variable length arrays, a vla should have an int variable as array size. In this case [] is used as pointer notation in the C sense, the memory itself is allocated in verificationpacket.cpp by function AllocatePacket(), whose definition I wasn't able to find, my cpp-fu is weak.

The fix I can see is just replacing entries[] with *entries as per attached patch, it succesfully built libpar2 on my Xcode 4.3 system. I tried port par2's test scripts as well, 3 out of 6 failed but I don't know if this is due to the upward fix. I'm leaving the review to someone who knows par2 or can actually test it on a real case.

This could be a clang issue if that syntax is recognized as variable length arrays while in fact it is not.

Changed 12 years ago by anddam (Andrea D'Amore)

Attachment: patch-port_libpar2.diff added

comment:11 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Yeah, with llvm-gcc it passes all 6 tests. With clang, it fails 4 out of 6 here...

comment:12 Changed 12 years ago by anddam (Andrea D'Amore)

The problem there is about port par2, are the failing tests due to this specific issue or are they clang related but not to the fixed libpar2?

If the latter we should commit libpar2 and close this ticket.

I think the [] syntax in that struct is just a C pointer and the patch is fine, but I'd like to get a specific review about that. Anyone?

comment:13 Changed 12 years ago by neverpanic (Clemens Lang)

JFYI, this is the reason the nzbget port is set to use llvm-gcc-4.2. It would be great if this was fixed, so I can drop the compiler change.

As to the question, yes type name[] is equivalent to type *name in this case as far as I know. Please commit the patch.

comment:14 Changed 12 years ago by neverpanic (Clemens Lang)

Alternatively, one could probably use type name[0].

comment:15 in reply to:  12 ; Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Replying to and.damore@…:

The problem there is about port par2, are the failing tests due to this specific issue or are they clang related but not to the fixed libpar2?

I don't know. There are no tests in libpar2, so I was using par2's tests to verify both since they share so much code.

comment:16 in reply to:  15 Changed 12 years ago by anddam (Andrea D'Amore)

Resolution: fixed
Status: newclosed

Replying to cal@…:

JFYI, this is the reason the nzbget port is set to use llvm-gcc-4.2. It would be great if this was fixed, so I can drop the compiler change. As to the question, yes type name[] is equivalent to type *name in this case as far as I know. Please commit the patch.

Committed r93338, ticket closed.

comment:17 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Port: par2 added
Resolution: fixed
Status: closedreopened
Summary: libpar2 won't build with clang++libpar2 and par2 fail 'make check' with clang++

And reverted in r93350 ...

comment:18 Changed 12 years ago by jmroot (Joshua Root)

Cc: arno+macports@… julian@… added

comment:19 in reply to:  14 Changed 12 years ago by jmroot (Joshua Root)

Replying to cal@…:

Alternatively, one could probably use type name[0].

This is what FreeBSD does.

comment:20 in reply to:  17 ; Changed 12 years ago by anddam (Andrea D'Amore)

Replying to jeremyhu@…:

And reverted in r93350 ...

Did you revert due to par2 failing tests?

comment:21 in reply to:  20 Changed 12 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Replying to and.damore@…:

Replying to jeremyhu@…:

And reverted in r93350 ...

Did you revert due to par2 failing tests?

Yes, as mentioned in the Portfile comment, the commit message, and this ticket ;)

comment:22 Changed 6 years ago by jmroot (Joshua Root)

Resolution: fixed
Status: reopenedclosed

Presumably resolved by r141223 and r141225.

Note: See TracTickets for help on using tickets.