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

`redundant build-depends entry` for convenience library with C(++) sources #40

Closed
OlivierSohn opened this issue Jul 3, 2018 · 10 comments

Comments

Projects
None yet
2 participants
@OlivierSohn
Copy link

commented Jul 3, 2018

Hello,

I recently came across this corner case: in this cabal file, there is a ("main") library, and a convenience library which just builds C++ sources.

The main library declares the convenience library as a dependance, so that the c++ objects built by the convenience library are available at link time for projects that depend on the main library.

If I remove this dependency, the main library builds, but the projects that depend on this main library have "symbol not found" link errors.

weeder detects the convenience library as a redundant build-depends entry of the main library.

In a way, I think this is a bug, and weeder should consider that dependencies on convenience libraries building C / C++ sources are never redundant (considering that here the semantics of the dependency is "aggregate build results").

What do you think?

Thanks!

PS: The reason I need separate libraries is that one library builds c (for a .hsc file), the other builds c++, and build options are not compatible (and cxx-sources / cxx-options is not yet available in the version of cabal used!).

@OlivierSohn OlivierSohn changed the title `redundant build-depends entry` for convenience library with c(++) sources `redundant build-depends entry` for convenience library with C(++) sources Jul 3, 2018

@ndmitchell

This comment has been minimized.

Copy link
Owner

commented Jul 4, 2018

My inclination is to say you should use weeders ignore feature here. I could say that any library that links to C is not eligible for being detected as redundant, but I can only do that within a package file (you don't know if you are depending on bytestring just for its C functions), and it's a bit of a special case. Furthermore, people should generally declare and wrap their C functions in one package, so as the C interface doesn't leak too far.

I can understand that it's the right thing in this case, but usually it isn't.

@OlivierSohn

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

Yes it makes sense. I'll use the ignore feature! Thanks.

@OlivierSohn OlivierSohn closed this Jul 4, 2018

ndmitchell added a commit that referenced this issue Jul 4, 2018

@ndmitchell

This comment has been minimized.

Copy link
Owner

commented Jul 4, 2018

Thanks, I added it as a false positive in 18e8639 so the knowledge isn't lost.

@OlivierSohn

This comment has been minimized.

Copy link
Author

commented Jul 4, 2018

While reading the false positive commit, and re-reading your previous message I see that maybe there is a misunderstanding: The C functions are not used directly by the other packages, they are called only from the library of the same package. But at link-time, the executable in the other package needs the objects corresponding to the C-functions. (in a way we can say that the other project uses the compiled binary object of the function "at link time", because a library it depends on uses it, but it doesn't use it directly)

@ndmitchell

This comment has been minimized.

Copy link
Owner

commented Jul 5, 2018

Indeed, I'm confused. Can you tell me the actual weeder error, so I have the names of what it is saying is redundant where?

@OlivierSohn

This comment has been minimized.

Copy link
Author

commented Jul 5, 2018

Yes, the corresponding .weeder.yaml file is here. This is the relevant part:

- package:
  - name: imj-audio
  - section:
    - name: library
    - message:
      - name: Redundant build-depends entry
      - depends: imj-audio-cxx
@ndmitchell

This comment has been minimized.

Copy link
Owner

commented Jul 7, 2018

From what I can see, that does match what I'm saying. Concretely, you have imj-audio-cxx which provides C functions, which aren't called by Haskell functions in imj-audio-cxx (because that library doesn't have any Haskell code in it), but are presumably called directly by the imj-audio library? That corresponds to a library providing C functions which are used directly from another library. Where have I gone wrong? (Ignoring the C vs C++ difference, and just using C to refer to either.)

@OlivierSohn

This comment has been minimized.

Copy link
Author

commented Jul 7, 2018

You're right, I misinterpreted: when you wrote library, I was thinking "main library" (i.e imj-audio here) but in fact library referred to the convenience library (imj-audio-cxx).

Sorry for the noise :)

@ndmitchell

This comment has been minimized.

Copy link
Owner

commented Jul 7, 2018

No problem - I appreciate the report, and the subsequent clarifications - I certainly learnt something, and I think we've distilled a nice case where Weeder is wrong.

@OlivierSohn

This comment has been minimized.

Copy link
Author

commented Jul 7, 2018

And I've learned about the ignore feature, I'm almost ready to use weeder in my CI script now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.