[PATCH wayland] wayland-egl: Ignore underscored symbols in ABI check

[Emil Velikov]

On 20 March 2018 at 14:50, Derek Foreman <derekf at osg.samsung.com> wrote:
> On 2018-03-20 07:11 AM, Daniel Stone wrote:
>> On 20 March 2018 at 11:55, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> On 20 March 2018 at 11:46, Daniel Stone <daniel at fooishbar.org> wrote:
>>>> Sure. As on IRC though, we definitely need to add at least _ftext for
>>>> MIPS anyway:
>>>> https://gitlab.gnome.org/GNOME/glib/commit/ad12142943e0f20ed9583c9d6bf50f6262110c74
>>>>
>>>> And probably some more for ARM toolchains using other linkers:
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474k/pge1362066045758.html
>>>>
>>>> Doing a quick check across all the architectures on Debian shows that
>>>> your updated list is also missing _fbss, _fdata, and _ftext. So we'd
>>>> need to respin for those as well, but I think at this point the
>>>> inconvenience of maintaining a list of every linker's implementation
>>>> detail on every platform, outweighs the risk of an exported
>>>> underscore-prefixed symbol slipping through review.
>>>
>>> I think you're preemptively worried about this. Plus there's no need
>>> to add every symbol a search could find.
>>>
>>> Even then, ask yourself the question:
>>> Which is better - updating an ugly looking list once in a blue moon or
>>> having apps use internal API?
>>>
>>> As said above: _if_ it turns out to be labour intensive - fine remove it.
>>> We've already spend more time researching than what the [expected]
>>> maintenance for 1 year would be ;-)
>>
>> It all depends on how you view scope and probability.
>>
>> I don't agree my concern is preemptive: we've just shipped a beta
>> where 'make check' fails on a few architectures, and the proposed fix
>> (adding several symbols to the platform list) will still fail on
>> architectures Debian ships on, until it also adds _fbss, _fdata and
>> _ftext. I think my suggestion is reasonable: it fixes the very real
>> problem, in a way which avoids expanding the scope of our ABI checker
>> to include the implementation details of every linker/architecture
>> pair that people use Wayland on. The cost of this is the probability
>> that someone manages to add a new underscored symbol marked with
>> WL_EXPORT into a library which is currently 105 LoC, and have no-one
>> notice that in review. I think that's a reasonable tradeoff.
>
> I'm inclined to agree with Daniel here.
>
> Someone managing to sneak a WL_EXPORT on a symbol starting with a _ past
> review seems reasonably unlikely.
>
> And after reading this thread I'm still not entirely sure we have a
> complete list of what should be on that whitelist, and there are only a
> limited number of systems I can test build on right now.
>
Sharing a few ideas that are less obvious. Having the partial list
allows you to:
 - see active developers/users - be that by merging their patches or
skimming through the report
 - have contact point for people with unusual platforms/setups
 - is fairly trivial newbie task ;-)

-Emil