python / cpython

The Python programming language
https://www.python.org/
Other
59.72k stars 28.94k forks source link

OSX patches - review wanted #34900

Closed jackjansen closed 22 years ago

jackjansen commented 22 years ago
BPO 448261
Nosy @loewis, @jackjansen
Files
  • @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: ```python assignee = 'https://github.com/jackjansen' closed_at = created_at = labels = ['build'] title = 'OSX patches - review wanted' updated_at = user = 'https://github.com/jackjansen' ``` bugs.python.org fields: ```python activity = actor = 'jackjansen' assignee = 'jackjansen' closed = True closed_date = None closer = None components = ['Build'] creation = creator = 'jackjansen' dependencies = [] files = ['3490'] hgrepos = [] issue_num = 448261 keywords = ['patch'] message_count = 4.0 messages = ['37176', '37177', '37178', '37179'] nosy_count = 2.0 nosy_names = ['loewis', 'jackjansen'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue448261' versions = [] ```

    jackjansen commented 22 years ago

    I'm posting these here because I'm not a configure-guru and I'd like someone to check that I'm doing reasonable things.

    The patch does three things:

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 22 years ago

    Logged In: YES user_id=21627

    On --with-dyld: If --with-dyld is the only choice on the Mac, then it should not be set implicitly. Instead, support for not having it should be removed. I.e. in all places that check for with_dyld, the Darwin case should be split out, and the resulting features should be automatically activated. E.g.

        Darwin/*|next/*) 
        if test "$ns_dyld"
        then LDSHARED='$(CC) $(LDFLAGS) -bundle 

    -undefined suppress' else LDSHARED='$(CC) $(CFLAGS) -nostdlib -r'; fi if test "$with_next_framework" ; then LDSHARED="$LDSHARED \$(LDLIBRARY)" fi ;;

    should change to

        Darwin/*)LDSHARED='$(CC) $(LDFLAGS) -bundle 

    -undefined suppress' LDSHARED='$(CC) $(LDFLAGS) -bundle -undefined suppress' next/*) if test "$ns_dyld" then LDSHARED='$(CC) $(LDFLAGS) -bundle -undefined suppress' else LDSHARED='$(CC) $(CFLAGS) -nostdlib -r'; fi if test "$with_next_framework" ; then LDSHARED="$LDSHARED \$(LDLIBRARY)" fi ;;

    As a result, --with-dyld becomes a next-only thing. It may be that --with-next-framework is also meaningless on Darwin, in which case the support for specifying it on Darwin should be removed. So I disapprove the part of the patch that just sets with_dyld.

    On Mac/Python, I think the hierarchy is backwards, it should be Python/Mac instead. It also appears that only macglue.c is actually used on Darwin (I don't know whether the others are ever used). If that is the case, macglue.c should be moved to Python/, in which case the SRCDIRS change is not needed anymore, either.

    In any case, I think Mac should not be in SRCDIRS, since it does not contain any sources. It appears you listed it only to get Mac created before Mac/Python is created. Instead, mkdir -p should be used to create SRCDIRS.

    While you are at it, you should carefully review the other Next stuff whether it really applies to Darwin. E.g. why is it that we check for

    -f /System/Library/CoreServices/software_version

    Is this for Darwin only, or was there a NeXT release that had this but not /usr/lib/NextStep/software_version? If this is for Darwin only, I think the test should be removed, with the Darwin code in it. I suppose that --with-next-arch is not supported on Darwin, is it?

    jackjansen commented 22 years ago

    Logged In: YES user_id=45365

    I'll get rid of the --with-dyld on Darwin, and I'll split macglue.c into two files, one for both OSX and OS9 going into Python, the other (only OS9) remaining in Mac/Python. 1 or 2 .h files will also move from Mac/Include to Include.

    That means that for unix-Python the whole Mac subtree will only be used for builds of extension modules. Eventually this subtree should probably be split into a section that is MacPython-only and a section that is used for both MacPython and unix-Python.

    --with-next-framework is definitely important on OSX, and also optional. After this patch is checked in there'll be a lot more coming that actually creates the frameworks.

    As to --with-next-arg: it is currently unused on OSX, but all the infrastructure is still there, so I'd say we leave it in.

    As to the other NeXT stuff: I haven't a clue. It's been 10 years since I saw a next machine, and that only from 2m distance. I try to be as conservative as I can, and not break NeXT support with the stuff I do for OSX, but I have no way of checking.

    jackjansen commented 22 years ago

    Logged In: YES user_id=45365

    Checked it in with Martin's suggestions.