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

[Derek Foreman]

On 2018-03-20 10:02 AM, Emil Velikov wrote:
> 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

I don't imagine we'll be adding new API to that library frequently
enough to mine that data productively.

>  - have contact point for people with unusual platforms/setups

By defaulting to a broken build state for unusual platforms, do you
really think we'll attract many contact points?

>  - is fairly trivial newbie task ;-)

I don't think we should be going out of our way to generate problems for
newbies to solve.  There are plenty of worthwhile tasks for new
participants already.

I've confirmed that we once again pass make check on arm (something I
really should've done prior to the beta release.  sigh.) and have pushed
this patch.

Thanks,
Derek

> 
> -Emil
>