Обсуждение: installdir patch for win32
For application to HEAD, following community review. Supplement to previous win32 patch for dfmgr. Win32 replacement for configure time constants like PKGLIBDIR. --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see <a href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em ailpolicy.html</a>
Вложения
Claudio Natoli wrote: > For application to HEAD, following community review. > > Supplement to previous win32 patch for dfmgr. Win32 replacement for > configure time constants like PKGLIBDIR. + /* + * Determine the directory of installed files, on the assumption that + * win32 will have a common root directory for all pgsql subdirectories + * (eg. lib, bin, share, etc). + */ What happens if this assumption does not hold?
Peter Eisentraut said: > Claudio Natoli wrote: >> For application to HEAD, following community review. >> >> Supplement to previous win32 patch for dfmgr. Win32 replacement for >> configure time constants like PKGLIBDIR. > > + /* > + * Determine the directory of installed files, on the assumption that > + * win32 will have a common root directory for all pgsql > subdirectories + * (eg. lib, bin, share, etc). > + */ > > What happens if this assumption does not hold? > > Things will break ... The problem is that we need to be able to create a binary installer for Windows where the location is not known until install time. This is common Windows practice, and Windows admins rightly hate packages that have hardcoded install paths. But to do that we need to be able to make a couple of assumptions about things which in our current setup are determined at configure time. In practice it is not likely to be a big problem. This was discussed in win32-hackers, and the consensus was that this was a reasonable price to pay, IIRC. cheers andrew
Claudio Natoli <claudio.natoli@memetrics.com> writes: > Supplement to previous win32 patch for dfmgr. Win32 replacement for > configure time constants like PKGLIBDIR. This is exactly the sort of cruft I was hoping we would not get saddled with by a Windows port. If the problem is that we need PKGLIBDIR not to be frozen at compile time, then let's fix the problem for everybody, not add a pile of undocumented #ifdef WIN32 hacks. (And it is a problem for everybody; see for example http://archives.postgresql.org/pgsql-general/2004-03/msg00894.php which boils down to the complaint that pkglibdir shouldn't be frozen by configure. The RPM packagers would also be happier if they could make relocatable RPMs of Postgres.) Every "#ifdef WIN32" I see reduces my opinion of the quality of work being done for this port. regards, tom lane
Tom Lane wrote: >Claudio Natoli <claudio.natoli@memetrics.com> writes: > > >>Supplement to previous win32 patch for dfmgr. Win32 replacement for >>configure time constants like PKGLIBDIR. >> >> > >This is exactly the sort of cruft I was hoping we would not get saddled >with by a Windows port. > >If the problem is that we need PKGLIBDIR not to be frozen at compile >time, then let's fix the problem for everybody, not add a pile of >undocumented #ifdef WIN32 hacks. (And it is a problem for everybody; >see for example >http://archives.postgresql.org/pgsql-general/2004-03/msg00894.php >which boils down to the complaint that pkglibdir shouldn't be frozen >by configure. The RPM packagers would also be happier if they could >make relocatable RPMs of Postgres.) > >Every "#ifdef WIN32" I see reduces my opinion of the quality of work >being done for this port. > > > Tom, I think in defense of Claudio and company, it should be noted that there has been a desire not to disturb existing functionality more than necessary, and that has led fairly obviously to use of #ifdef with some liberality. The particular issue is not a bolt from the blue - it was at least discussed a couple of months ago on the -hackers-win32 list, IIRC. I agree we should fix the problem more generically, since as you point out it isn't WIN32 specific. cheers andrew
Andrew Dunstan wrote: > >Every "#ifdef WIN32" I see reduces my opinion of the quality of work > >being done for this port. > > > > > > > > Tom, > > I think in defense of Claudio and company, it should be noted that there > has been a desire not to disturb existing functionality more than > necessary, and that has led fairly obviously to use of #ifdef with some > liberality. Agreed. When we have issues with special socket restriction in signal handlers on the platform, and things like that, there isn't much we can do except #ifdef WIN32. I have been surprised how few #ifdef's we have needed for this platform. I do like to see a comment at the top of WIN32-specific code so we know what OS limtiation we are working around, and I think that has been done. In fact, here is a count of WIN32 references in each file. You will find the backend has very few references, with most in interfaces or /bin, which makes sense. --------------------------------------------------------------------------- 1 ./backend/access/transam/slru.c 1 ./backend/access/transam/xlog.c 1 ./backend/commands/copy.c 4 ./backend/commands/dbcommands.c 2 ./backend/libpq/md5.c 1 ./backend/libpq/pqcomm.c 3 ./backend/libpq/pqsignal.c 8 ./backend/main/main.c 5 ./backend/postmaster/pgstat.c 13 ./backend/postmaster/postmaster.c 6 ./backend/utils/fmgr/dfmgr.c 3 ./backend/utils/init/findbe.c 1 ./backend/utils/init/miscinit.c 4 ./backend/postgres 12 ./bin/initdb/initdb.c 1 ./bin/pg_resetxlog/pg_resetxlog.c 1 ./bin/psql/bcc32.mak 10 ./bin/psql/command.c 6 ./bin/psql/common.c 3 ./bin/psql/common.h 2 ./bin/psql/copy.c 5 ./bin/psql/describe.c 8 ./bin/psql/help.c 1 ./bin/psql/input.c 4 ./bin/psql/mainloop.c 1 ./bin/psql/mainloop.h 1 ./bin/psql/mbprint.c 4 ./bin/psql/print.c 1 ./bin/psql/prompt.c 7 ./bin/psql/startup.c 1 ./bin/psql/win32.mak 1 ./bin/scripts/common.c 4 ./bin/scripts/print.c 1 ./bin/scripts/mbprint.c 4 ./include/c.h 1 ./include/miscadmin.h 1 ./include/pg_config.h.win32 3 ./include/pg_config_manual.h 5 ./include/port.h 1 ./include/rusagestub.h 1 ./include/libpq/hba.h 3 ./include/libpq/pqcomm.h 3 ./include/libpq/pqsignal.h 1 ./include/port/win32.h 1 ./include/utils/elog.h 1 ./interfaces/ecpg/include/sqlca.h 1 ./interfaces/ecpg/pgtypeslib/dt.h 2 ./interfaces/libpgtcl/win32.mak 1 ./interfaces/libpq/bcc32.mak 2 ./interfaces/libpq/fe-auth.c 2 ./interfaces/libpq/fe-exec.c 1 ./interfaces/libpq/fe-lobj.c 1 ./interfaces/libpq/fe-misc.c 8 ./interfaces/libpq/fe-print.c 1 ./interfaces/libpq/fe-protocol2.c 1 ./interfaces/libpq/fe-protocol3.c 6 ./interfaces/libpq/fe-connect.c 8 ./interfaces/libpq/fe-secure.c 2 ./interfaces/libpq/md5.c 1 ./interfaces/libpq/libpqdll.c 2 ./interfaces/libpq/win32.c 1 ./interfaces/libpq/pqexpbuffer.c 1 ./interfaces/libpq/pqsignal.c 1 ./interfaces/libpq/win32.mak 3 ./interfaces/libpq/libpq-int.h 2 ./interfaces/libpq/noblock.c 4 ./interfaces/libpq/path.c 2 ./interfaces/libpq/thread.c 4 ./interfaces/libpq/libpq.so.3.2 4 ./interfaces/libpq/libpq.so.3 4 ./interfaces/libpq/libpq.so 1 ./port/crypt.c 3 ./port/dirmod.c 1 ./port/getrusage.c 2 ./port/noblock.c 4 ./port/path.c 1 ./port/pgsleep.c 4 ./port/sprompt.c 2 ./port/thread.c 1 ./port/open.c 2 ./utils/dllinit.c -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > If the problem is that we need PKGLIBDIR not to be frozen at compile > time, then let's fix the problem for everybody, not add a pile of > undocumented #ifdef WIN32 hacks. (And it is a problem for everybody; Happy to go with whatever you can suggest. However, will point out that this patch is refactoring for general use code which had already accepted and in the source base, and for which opinions had already been previously canvassed. > Every "#ifdef WIN32" I see reduces my opinion of the quality of work > being done for this port. At risk of getting (further? :-) on your bad side, IMHO that is a specious metric. Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see <a href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em ailpolicy.html</a>
Claudio Natoli <claudio.natoli@memetrics.com> writes:
> Tom Lane wrote:
>> Every "#ifdef WIN32" I see reduces my opinion of the quality of work
>> being done for this port.
> At risk of getting (further? :-) on your bad side, IMHO that is a specious
> metric.
Well, it's surely not the only interesting metric, but I don't think
it's specious. Every #ifdef poses a continuing load on future
maintainers, who have to look at that code and think whether they need
to worry about adjusting it when making nearby changes. To the extent
that you can avoid ifdefs in favor of cleaner solutions (such as
refactoring code, or solving a general problem instead of making a
platform-specific change in behavior), you'll have more readable and
more maintainable code.
I'm not expecting to see zero ifdefs --- certainly not in the port
modules ;-). But Bruce's search, further up in the thread, showed that
#ifdef WIN32's are sneaking into a lot of modules that probably
shouldn't have any platform dependencies. I don't think that's a good
sign. We should be working to keep those dependencies localized.
regards, tom lane
Tom Lane said: > > I'm not expecting to see zero ifdefs --- certainly not in the port > modules ;-). But Bruce's search, further up in the thread, showed that > #ifdef WIN32's are sneaking into a lot of modules that probably > shouldn't have any platform dependencies. I don't think that's a good > sign. We should be working to keep those dependencies localized. > I think it's a chicken and egg argument. Now that we are getting close we have a better idea of what needs to be wrapped in port-specific modules etc. (Bruce's count was slightly over - I ran a slightly more restrictive search and found 193 occurences of #ifdef WIN32 and friends.) I'll revisit some of my contributed code and see what can be eliminated. cheers andrew
On Thu, Mar 25, 2004 at 07:57:19PM -0500, Tom Lane wrote: > I'm not expecting to see zero ifdefs --- certainly not in the port > modules ;-). But Bruce's search, further up in the thread, showed that > #ifdef WIN32's are sneaking into a lot of modules that probably > shouldn't have any platform dependencies. Don't forget about the EXEC_BACKEND businness ... is there any chance those could refactored somehow? -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Amanece. (Ignacio Reyes) El Cerro San Cristóbal me mira, cínicamente, con ojos de virgen"
> I'm not expecting to see zero ifdefs --- certainly not in the port > modules ;-). But Bruce's search, further up in the thread, showed that > #ifdef WIN32's are sneaking into a lot of modules that probably > shouldn't have any platform dependencies. For the most part, I disagree (in fact, I was surprised to see how few there were). The bulk come from include, bin, interfaces, port... with almost all of the first three hits of these existing prior to the port. With regards to the backend files, a healthy number of these are pre-existing, or exist only to include/exclude header files + struct members, or simply avoid things that are unix specific. I recall a few potential culprits lurking in pgstat + postmaster (like the pipe() + win32_fork, which could be shipped off to /port), but nothing that would substantially impact the number of #ifdefs. > I don't think that's a good sign. We should be working to keep > those dependencies localized. On this, I agree, and if you can point me towards any that you find particularly obnoxious (on-list or otherwise), I'll gladly fix them. Seriously. From my POV, the WIN32 specific areas should be clear and obvious, and present no great maintenance challenge. This attitude, however, does not extend to some of the EXEC_BACKEND areas, but this is also where we are obviously the most hamstrung. As you probably remember, I've already undertaken to refactor this section of the backend startup code when the dust settles, but some degree of pain will be had here for as long as we choose to support platforms without fork(). Cheers, Claudio --- Certain disclaimers and policies apply to all email sent from Memetrics. For the full text of these disclaimers and policies see <a href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em ailpolicy.html</a>
With most of the Win32 code done, I expected this type of review to see how we could clean things up further. It would probably be helpful for folks to poke around and suggest cases where we can generalize or move things to /port or /backend/port. --------------------------------------------------------------------------- Claudio Natoli wrote: > > > I'm not expecting to see zero ifdefs --- certainly not in the port > > modules ;-). But Bruce's search, further up in the thread, showed that > > #ifdef WIN32's are sneaking into a lot of modules that probably > > shouldn't have any platform dependencies. > > For the most part, I disagree (in fact, I was surprised to see how few there > were). The bulk come from include, bin, interfaces, port... with almost all > of the first three hits of these existing prior to the port. > > With regards to the backend files, a healthy number of these are > pre-existing, or exist only to include/exclude header files + struct > members, or simply avoid things that are unix specific. > > I recall a few potential culprits lurking in pgstat + postmaster (like the > pipe() + win32_fork, which could be shipped off to /port), but nothing that > would substantially impact the number of #ifdefs. > > > > I don't think that's a good sign. We should be working to keep > > those dependencies localized. > > On this, I agree, and if you can point me towards any that you find > particularly obnoxious (on-list or otherwise), I'll gladly fix them. > Seriously. > > >From my POV, the WIN32 specific areas should be clear and obvious, and > present no great maintenance challenge. This attitude, however, does not > extend to some of the EXEC_BACKEND areas, but this is also where we are > obviously the most hamstrung. As you probably remember, I've already > undertaken to refactor this section of the backend startup code when the > dust settles, but some degree of pain will be had here for as long as we > choose to support platforms without fork(). > > Cheers, > Claudio > > --- > Certain disclaimers and policies apply to all email sent from Memetrics. > For the full text of these disclaimers and policies see > <a > href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em > ailpolicy.html</a> > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073