access/http: remove GCC visibility pragma from static libraries
Either we need to do it in all static libraries or none. And it should be none.
Merge request reports
Activity
changed milestone to %4.0
mentioned in merge request !3085 (merged)
added 1 commit
- afdf6c0e - access/http: remove GCC visibility pragma from static libraries
added Component::Input: network streams label
As mentioned in !3085 (merged), it seems legit to me since it's not C++ and it's not a shared library or executable.
Given the first pipeline, now I know when it's needed: when the library is shared (makes sense). We don't do it for the other static libraries. Not sure if it was ever needed for that library as it was static from the start: aa78e9d3.
- Resolved by Steve Lhomme
It's a bug that HTTP is static in the first place
added MRStatus::InReview label
If it's used at several place, I would personnaly prefer a shared library (with a proper header and proper VLC_API). Is it so bad to have more libraries sitting in the root directory?
Would the release master @jbk oppose it?
This is not really about the module though. Even if you were to add a different HTTP access module, other modules would still need a http API they can use for cases where the VLC access/stream abstraction is not suitable to express all needs of a module that wants to do more complex HTTP request, like talking to an API that needs custom headers and special response handling.
I think that Marvin's point is the same as mine: libvlc_http.la is not a module and is used by modules that don't necessarily want the vlc_stream part like access-out.
Edited by Denis CharmetModules only make sense if you have multiple implementations of the same API. One such case is TLS. This should not be the case here for HTTP though. We may have different APIs for the "same" functionality (as Thomas noted).
Other than that modules can be used to leave dependencies out of the core, but here we have no additional dependencies.
And the module subsystem is very unfriendly for unit tests, and for large API sets. So it would not only be useless, but outright harmful and annoying to make this a module.
And the module subsystem is very unfriendly for unit tests, and for large API sets. So it would not only be useless, but outright harmful and annoying to make this a module.
Not questioning the rest of the answer, as a side note:
Setting the test context is a bit unconventional, but it's not that different afterwards, without accounting tricks like libvlc_adaptive where the whole module is first wrapped as a convenience library to create unit test, and then linked into a module declaration. I've made multiple test samples where it's exploited, the last one being the simpler image.c test.
If this is really too much and too incompatible, I can try simplifying that even more.
mentioned in merge request !3101 (merged)
added 1 commit
- 33185c2a - access/http: remove GCC visibility pragma from static libraries