Changing private symbols to public (was: Re: CVS Update: xc (branch: trunk))

[Mike A. Harris]

On Wed, 2 Feb 2005, Daniel Stone wrote:

>> On Tue, 1 Feb 2005, Daniel Stone wrote:
>> >On Tue, Feb 01, 2005 at 12:35:03AM -0500, Mike A. Harris wrote:
>> >> I searched the mailing list, and I couldn't find any emails 
>> >> mentioning the rationale or approval for this change.  Was this 
>> >> discussed on a confcall or somesuch?
>> >
>> >No, it wasn't.
>> 
>> IMHO, such API changes should be discussed and concensus reached 
>> before any changes occur.
>
>It's not any change from the status quo.  Honestly, if it's private
>API, then by definition, libSM was never physically able to link to it.
>By the fact it was, means that it was public API.  This was adding a
>symbol and bumping the soversion to account for this fact.

Using this logic, every single symbol that isn't explicitly 
hidden, is public, and should now be moved to be a public symbol.  
That would pretty much expose most if not all internal symbols of 
all of the X libraries, as they're not hidden with 
attribute((hidden)).

That is a bogus argument.  All of the internal symbols in various 
libraries were _intended_ to be internal, and wether they are 
explicitly hidden or not, they should be treated as private, 
wether or not that is enforced by the compiler currently.

Otherwise, with your logic, where does one draw the line?


>> >> Could someone summarize the rationale for the change, and it's 
>> >> acceptance into CVS head?
>> >
>> >The rationale is simple: if another library is using it, it is not
>> >private API, no matter how many underscores you put in front of the
>> >symbol, and no matter that you put a comment saying 'this is private
>> >API'.  It's simply not.
>> 
>> I don't disagree with that.  X.Org needs to be enhanced to hide 
>> it's internal library symbols using gcc's attribute((hidden)) 
>> functionality (and potentially other compiler's similar 
>> functionality).  Jakub Jelinek wrote some stuff for XFree86 
>> before to do this which nobody ever bothered integrating for 
>> whatever reason.
>
>Christ.  I'd love to dig that up and integrate it.

Feel free.


>> >I made this change so we could actually delineate between what
>> >was and was not private API.  I don't know what the rationale
>> >for intruding on another library's allegedly private API was in
>> >the first place, but I just made it official: either it should
>> >be public, or libSM must not rely on it.
>> 
>> This indicates that there is indeed a problem, and that someone 
>> should investigate potential solutions for the problem, discuss 
>> the problem openly on the mailing lists, and get other people's 
>> thoughts on weighing the ups and downs of the different potential 
>> solutions.
>> 
>> Then, once concensus can be reached between the parties 
>> discussing the potential solutions, whichever solution is 
>> decided, can be approved and changed officially.
>> 
>> I don't think it is a good idea for random developers (whoever 
>> they are) to just go ahead and make API changes of this nature 
>> without consulting other developers who will be potentially 
>> impacted by the changes.
>
>Again, I challenge you: if it's private API, remove libSM's dependency
>on it.  Consider it a documentary clarification of existing fact.

You're missing my entire point.  You are looking at this entirely 
as a specific issue of this one specific change.  I do not know 
wether or not the libSM dependancy can be removed or not, and I 
don't really care to be honest.

My concern, is that random developers are making changes to the 
intended visibility of internal X library symbols on a personal 
whim without any pre-discussion on xorg mailing lists, and 
without any peer review.  I consider this practice to be 
unacceptable by anyone, without discussing the changes on the 
xorg mailing lists FIRST.  If the change truly is safe and sane, 
and makes total sense, then there isn't likely to be much 
objection is there?  Please ignore the specific change you just 
made when answering this question, and look at the bigger picture 
of anyone randomly changing any other symbol in any other 
library.

Unless there is a standard process that these type of changes 
have to go through, then people will end up eventually making 
changes that break compatibility, or cause other damage that 
can't easily be fixed in the future.

If other X.Org developers feel the same way as I do about this
concept, then I would like to hear them stand up and say so.

If other developers think I am wrong, and that it is ok for
anyone with CVS commit access to change the visibility of library
symbols without discussing them on the mailing lists first, then
I would like them to stand up and say so.

If other developers are completely indifferent and think everyone 
can judge for themselves when making a change, then I'd like them 
to stand up and say so also.



>> I think we need to establish some CVS policies to ensure that any 
>> changes of this nature are adequately discussed and agreed upon 
>> by multiple developers via peer review of some sort.
>> 
>> Again, my point isn't necessarily about the specific change, but 
>> rather the fact that it occured without discussion.
>> 
>> Comments/feedback from anyone and everyone appreciated.
>
>I'm happy to stay out of the xorg pissing pond, as it were, and stick
>to xlibs if that would make you happy, but I honestly don't think that
>changes of this nature impact anything at all.  If you want to remove
>it from the public API: well, for one, you're too late; for another,
>remove libSM's dependency on it.  As I've stated before, this was
>merely a documentary clarification of existing fact.

I wasn't suggesting that you stay out of xorg.  What I'm
suggesting, is that you discuss these type of changes with other
developers in the open public forums ahead of time and see if 
people have a general concensus that the changes are sensible, or 
if they have specific concerns about them.  This is not about the 
specific change, but about the idea of someone changing something 
of this nature without discussing it previously with others.


>It's not introducing new symbols, it's not breaking backwards
>compatibility, it's not doing anything radical.

You're focussing on the specific change, rather than what I'm 
talking about.


>It's correcting an unfortunate misstatement somewhere along the
>line.  When libSM started using _IceGetPeerName, the person who
>did that made a conscious decision to move that symbol into the
>public API.  It has not been private API since that point, and
>the existing situation served only to lie about its true
>visibility.  Honestly, if you want this fight, find whoever
>started polluting libSM with ICE internal API and take it up
>with them, but I don't care enough, and I'm beginning to wish
>that I had never bothered attempting to enforce sanity in the
>monolithic tree, and just staying with the modular tree instead.

If you choose to see my concerns as fighting you, then you're 
being overly defensive.  Any project of this size, with as many 
developers as X.Org has contributing to it, has to have some 
sanity to it all.

Unfortunately, there are no real written CVS policies that detail
acceptable versus unacceptable changes, or detail what should be
discussed amongst developers first and concensus reached before
changes occur.  Without having any guidelines or controls in 
place, every developer is completely free to make whatever random 
changes they see fit, and hope nobody notices.

I totally support project openness, modernization, am quite
accepting of changes, and am also all for easy access to CVS
write priveledge to the masses including you, in order for it to
"work" and "work well" for the project as a whole, and all of the
participants involved, is by having good communication between
each other, good working relationships, and erring on the side of
caution by discussing certain types of changes with others prior
to making them.  If you can forget about this particular change
for a minute, would you agree that these are in general, good
ideas for people to follow?



-- 
Mike A. Harris, Systems Engineer - X11 Development team, Red Hat Canada, Ltd.

IT executives rate Red Hat #1 for value:  http://www.redhat.com/promo/vendor