Re: [PATCHES] installdir patch for win32
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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] installdir patch for win32
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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] installdir patch for win32
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 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] installdir patch for win32
Claudio Natoli [EMAIL PROTECTED] 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 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] installdir patch for win32
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 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] installdir patch for win32
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
[PATCHES] 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 libdirs.patch Description: Binary data ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
