Split-out ntmodule.c

[loewis]

BPO	592529
Nosy	@gvanrossum, @tim-one, @loewis, @mhammond
Files	ntmodule.c posixmodule.diff build.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/loewis'
closed_at = <Date 2002-08-11.17:43:24.000>
created_at = <Date 2002-08-08.10:12:33.000>
labels = ['OS-windows']
title = 'Split-out ntmodule.c'
updated_at = <Date 2002-08-11.17:43:24.000>
user = 'https://github.com/loewis'

bugs.python.org fields:

activity = <Date 2002-08-11.17:43:24.000>
actor = 'loewis'
assignee = 'loewis'
closed = True
closed_date = None
closer = None
components = ['Windows']
creation = <Date 2002-08-08.10:12:33.000>
creator = 'loewis'
dependencies = []
files = ['4489', '4490', '4491']
hgrepos = []
issue_num = 592529
keywords = ['patch']
message_count = 5.0
messages = ['40862', '40863', '40864', '40865', '40866']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'tim.peters', 'loewis', 'mhammond']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue592529'
versions = []

[loewis]

This patch moves the MS_WINDOWS code from
posixmodule.c into ntmodule.c. The OS/2 code is left in
posixmodule.c.

I believe this patch significantly improves readability of
both modules (posix and nt), even though it adds a
slight code duplication. It also gives Windows
developers the chance to adjust the implementation
better to the Win32 API without fear of breaking the
POSIX versions.

Attached are three files: the ntmodule.c source code,
the posixmodule.c diff, and the pcbuild diff. Since the
patches will outdate quickly, I'd appreciate if that patch
could be accepted or rejected quickly.

Randomly assigning to Tim.

[tim-one]

Logged In: YES
user_id=31435

I'm -0, so assigning to Guido for another opinion. I expect
this will actually make it harder to keep the os interface
consistent and working across platforms; e.g., somebody
adds an os function in one module but forgets to add it in the
other (likely because they don't even know it exists); a
docstring repair shows up in one but not both; a largefile fix in
one doesn't get reflected in the other; etc. Apart from the
massive Windows popen pain, there are actually more
embedded PYOS_OS2 #ifdefs in posixmodule.

[gvanrossum]

Logged In: YES
user_id=6380

I'm +0.5 on this. Can you bring this up on python-dev to see
if there are different viewpoints? I think the resulting
code will be easier to maintain; new stuff added at this
point is more likely to be unique to Unix anyway (or unique
to Windows).

I wonder if the os2 code shouldn't be moved to its own file
as well (I think Andrew MacIntyre maintains that port, right?).

There are still a bunch of #ifdefs in the nt code. Are those
really variable across Windows versions or compilers? If
not, I'd suggest to expand those.

[mhammond]

Logged In: YES
user_id=14198

I too am -0 on this, for the exact reasons Tim gives.

I think a better strategy would be to:

• Move most of the #ifdef cruft at the top of the source file to
  pyconfig.h and/or pyport.h, making these "HAVE_" macros
  consistent with the rest of the HAVE_ cruft. Most #defines in
  this module them move simply to HAVE_FEATURE rather
  then IS_SPECIFIC_OS

• Move popen, and possibly one or 2 other Windows specific
  functions to a separate source file. Possibly repeat for OS/2,
  but it is not clear there are huge OS/2 slabs of code that
  would make it worthwhile.

This would be a good start, reflects the existing
posixmodule.c comment:
/* Various compilers have only certain posix functions */
/* XXX Gosh I wish these were all moved into pyconfig.h */

and does not preclude a more aggressive split in the future.

However, my opinions on this are not strong enough to try
and -1 it :)

[loewis]

Logged In: YES
user_id=21627

Ok, I withdraw this patch.