Hi Nils, I hope I will answer correctly because it's pre-Christmas work, so my memory was cleaned a bit already :-)
Nils Goroll p??e v Ne 04. 01. 2009 v 20:49 +0100: > (this is minor edit of a message I have already to Zdenek in private) > > Hi Zdenek and all, > > I am sorry that it took me so long before getting back to you on this. I'll > go > through your mails in chronological order. > > > 1) Makefile.master > > > > You added variables which you are not use. Because they are not used and > never will (/usr/sfw is obsolete), you should remove them. > > Then please remove everything which is obsolete. I have not introduced the > CFGSFW Variables and I try to make my changes not to be destructive with > regard > to existing code. I cannot know what is regarded as obsolete if that is not > obvious from the existing code or documented elsewhere in the public. > > The CFGSFW Variables are in use also outside the postgres build and quagga is > a > recent addition, so I find it not very obvious to consider it obsolete: > > haggis:~/Devel/sfw-src-b100-20080930$ find . -xdev -name Makefile.sfw | grep > -v > postgres| xargs grep CFGSFW > ./usr/src/cmd/quagga/Makefile.sfw:CFGINFODIR= $(CFGSFWINFO) > ./usr/src/cmd/quagga/Makefile.sfw:CFGMANDIR= $(CFGSFWMAN) > ./usr/src/cmd/quagga/Makefile.sfw:CFGINCLUDEDIR= $(CFGSFWINCLUDE) > ./usr/src/cmd/quagga/Makefile.sfw:CFGLIBDIR= $(CFGSFWLIB) > ./usr/src/cmd/sma/Makefile.sfw: --prefix="$(CFGSFW)" \ > ./usr/src/cmd/sma/Makefile.sfw: > --with-ldflags="-R$(CFGSFWLIB)/$(MACH64) > $($(MACH64)_XARCH)" \ > ./usr/src/cmd/sma/Makefile.sfw: --libdir=$(CFGSFWLIB)/$(MACH64) > > What I have done is this: > > * Add missing variables for paths I needed to refer to > > +UCBINSTALL= /usr/ucb/install > +CFGINCLUDE= $(CFGPREFIX)/include > +ROOTINCLUDE= $(ROOT)$(CFGINCLUDE) > > * Add variables to existing sections which existed in other sections > to make the various sections consistent to earch other: > > +ROOTLOCALE= $(ROOT)$(CFGLOCALE) > +ROOTINFO= $(ROOT)$(CFGINFO) > +ROOTMAN= $(ROOT)$(CFGMAN) > > +ROOTSFWBIN32= $(ROOT)$(CFGSFWBIN32) > +ROOTSFWBIN64= $(ROOT)$(CFGSFWBIN64) > +ROOTSFWLIB64= $(ROOT)$(CFGSFWLIB64) > > * Cleaned up the existing CFGSFW section (if it's there, it should be clean > and > well structured. If it's obsolete, it should be removed) > > * Corrected a typo: > -ROOTLIBEXEC= $(ROOT)$(CFGLIBEXEC) > +ROOTSFWLIBEXEC= $(ROOT)$(CFGSFWLIBEXEC) > > I do think all of these changes are justified in order to improve the > structure > and clean up the Makefiles. > No problem with this cleanup if: a) all those variables are used in gate somewhere b) if they are in separate change, because many of them have no impact on your PostgreSQL /usr/sfw directory is obsolete based on some public PSARC case. If it is not described on SFW webpages, that's mistake and should be fixed there. Even if it is obsolete, it still survives, because it cannot be removed in an hour (many deliverables exist there yet). I have nothing against changes in Makefile.master if they have big benefit (like UCBINSTALL), but redefinition of CFGSFW* just because of /usr/sfw and adding new, unused, is wasting of time in my opinion. I understand what you are trying to do. SFW is not in the best shape, its infrastructure is improving just these days (finally), but I would prefer to make your changes more tied to PSQL build process. Maybe you can discuss this particular change with Norm Jacobs who is trying to clean up SFW I think. You can disagree, of course, and we can leave it as it is done by you now :-) > But anyway, please remove anything which is obsolete - that is none of my > business. > > > We discussed also Portion copyright here in this file. We know that by > process it is your right to have it here, but it seems to us a too little > change > :-) similar to targetdir file. We prefer to do not put it here, but it is up > to you. > > I can do without the copyright there, but I would like to remind you that > there > is not much benefit one gets from involvement into the lengthy, bureaucratic > and > complex Opensolaris contribution process so considering the time I need to > spend > on developing a fix like this one and doing several reviews I would think > that > at least getting some credit in the form of a partial copyright would be > morally > justified. > Of course, the credit is morally justified and we prefer to add your copyright everywhere else. The reason we would prefer to avoid it in Makefile.master - it is so generic file and with tons of "contributed by" it will result in 99% lines of "contributed by" and 1% of real data in 20 years and noone can be removed. Btw. it's off topic, but contribution to OpenSolaris isn't so lengthy and bureaucratic in comparison to other big projects, just a bit more standardized and visible, and we are learning also :-) Anyway, your contribution will be visible in SCM history forever and if you want even after my reason to have it in Makefile.master, then it can be there. > > > 2) Targetdirs > > > > Targetdirs is neverending story. Nobody knows if all directory have to be > there or not. By our discussion and what I remember from Mike Sulivan email, > There should be only directories which are not created automatically. > Personally > I think there should be only directories which are not created by sfw gate > like > /var /usr and so on. > > > > Please, remove all modifications here. They are useless. > > Then please remove *everything* which is useless. I have not introduced this > file and as long as it's there (what I have found when I started working on > this > was already more than what is not being created automatically), I adjust to > what > exists. > Then open new CR requesting removal of unneeded lines in Targetdirs. It has nothing to do with PostgreSQL. Yes, it should be documented somewhere on opensolaris.org (Hint for SFW project leaders). I see no reason to add additional lines there when they aren't needed, it will just increase the mess. > > 3) METADATA > > > > Please, remove # comments first three lines. > > Why would you want to remove an extremely helpful comment which directs every > developer to essential documentation regarding cryptic field names (like OSR, > for instance)? > Because it is not correct place where to document such thing. It can be in some global README in gate or on opensolaris.org Why should these 3 lines be in every METADATA file in gate? It is wasting of resources. > NO, absolutely not. It has cost me a couple of hours or so to find out what > all > of this means and to start some public documentation, and I do not want the > next > developer working on this to be forced to do the same again. > I agree, it is bad if documentation is missing. But you placed the documenation on wrong place. So, remove it, please. And push SFW leaders to improve documentation: http://opensolaris.org/os/project/sfwnv/leaders/ Or you can ask them for write access to wiki and improve it yourself. It will have better impact for everybody, because people will find it in some expectable place, not in one file somewhere in gate. > > 4) remove unset PROFILE > > OK > > > 5) please, remove FIXME from install.sfw or fix it :-) > > I hope it will be fixed in 8.4 integration. > > I cant fix everything at once, in particular if it takes months to get > something > integrated. > > I can fix this later, and for the time being I would like to keep the > reminder > in there. > OK. I am not fan of 'FIXME' lines in code. But if Zdenek and Petr will agree with it, then I don't see it as showstopper :-) > > 6) libpqxx/Makefile.sfw > > > > libpqxx is not possible to make separately. It is annoying. We made > conclusion that Makefile.sfw should be "makeable". Probably best way how to > do > it is to include some file which will contains path settings. > > OK, I can care about this later. Please get this thing done at least and I > will > be happy to make further contributions with further improvements. > > I Think I have already cleaned up libpqxx/Makefile.sfw significantly. > In such case, could you document it in that file, please? Just like you did with FIXME upper. > > 7) psqlrc issue > > > > After long and hot discussion we made a decision to do not accept this > change > for 8.3 at all. The reason is that it change behavior and there is no easy > and > safe way how to implemented and does not break current installations. > > See the public discussion. I don't agree with your argument, but you'll have > the > last word anyway. > The last word will have CRT advocate :-) I must say the original implementation was wrong and we spent many minutes to discuss clean way, but the key reason for "not do it now" is in something which has some interface impact (even if not documented now) and should be ARCed. And I would prefer to not prolong this PSQL makefile cleanup for not so important part. > > Do you want me to prepare another webrev or can you go ahead with the > integration on the basis of what we have discussed? > Please, fix those parts which were sorted out already, in way how you would prefer. I think Zdenek can make nightlies then. Best regards, Milan
