classification
Title: makesetup interprets macros -DA=B as a Make variable definition
Type: compile error Stage: patch review
Components: Build Versions: Python 2.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, eric.smith, wdebruij
Priority: normal Keywords: needs review, patch

Created on 2010-02-16 00:44 by wdebruij, last changed 2017-11-08 13:26 by Michael Evans.

Files
File name Uploaded Description Edit
pydiff wdebruij, 2010-02-16 00:44 patch
Pull Requests
URL Status Linked Edit
PR 4338 open Michael Evans, 2017-11-08 13:26
Messages (4)
msg99378 - (view) Author: Willem de Bruijn (wdebruij) Date: 2010-02-16 00:44
(this is very low priority: an issue in a nonstandard build)

I am porting Python 2.6.4 to a system with only partial Posix support that requires static linking for all modules. To enable sqlite, I added the line 

_sqlite3 _sqlite/cache.c _sqlite/connection.c _sqlite/cursor.c _sqlite/microprotocols.c _sqlite/module.c _sqlite/prepare_protocol.c _sqlite/row.c _sqlite/statement.c _sqlite/util.c -DMODULE_NAME='"sqlite3"' -IModules/_sqlite ...

to Modules/Setup, but found that it would not end up in the main binary. Instead of adding it to MODOBJS, makesetup put it in BASEMODLIBS

The problem occurs in Modules/makesetup, line 131, which matches all lines containing "=" and treats them as Make definitions. In this case, it matches the equal sign in the C macro MODULE_NAME. 

The problem goes away when I tighten the check at that line:

-		*=*)	DEFS="$line$NL$DEFS"; continue;;
+		[[:alpha:]]*=*) DEFS="$line$NL$DEFS"; continue;;

(see the attached file for the trivial patch in diff -Nur format)

This patch comes with a big warning: my sh-regex foo is very poor. I wanted to write a test that only accepted ^[:alpha:][^ ]*=*, but that
did not work. The above did, but I cannot oversee all consequences.

cheers,

  Willem
msg112904 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-04 21:47
msg99378 is self explanatory.  Do we accept the patch or not?
msg117564 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-09-28 23:47
"man sh" does not list [[:alpha:]]* as an accepted pattern. This is a glob pattern, not a regular expression.
While the proposed patch may work for some environments, we should find a more compatible way.  Maybe a new case
   "[ ]*=") ;;
(a space somewhere before the '=') could be used before to prevent '*=*' from matching the sqlite line.
msg117569 - (view) Author: Willem de Bruijn (wdebruij) Date: 2010-09-29 00:59
Good call. The posix regular expression character class [[:alpha:]] is apparently not as universally supported as I thought. Posix mandates it for filepath expansion[1], but that's not of much help on older and non-compliant systems. However, from what I can gather even standard Bourne accepts plain character classes ([a-z]) in its filename expansion[2]. If so, a simple fix is to rewrite it as 

-		*=*)	DEFS="$line$NL$DEFS"; continue;;
+		[a-zA-Z]*=*) DEFS="$line$NL$DEFS"; continue;;


On the other hand, your solution should also work and is actually more robust, so I'd go with that.

  willem

--
references:

[1] http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_13_01
[2] http://steve-parker.org/sh/bourne.shtml
History
Date User Action Args
2017-11-08 13:26:11Michael Evanssetstage: patch review
pull_requests: + pull_request4292
2014-02-03 19:41:40BreamoreBoysetnosy: - BreamoreBoy
2010-09-29 00:59:27wdebruijsetmessages: + msg117569
2010-09-28 23:47:02amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg117564
2010-08-04 21:47:19BreamoreBoysetnosy: + BreamoreBoy
messages: + msg112904
2010-02-16 01:03:39eric.smithsetnosy: + eric.smith
2010-02-16 01:03:10eric.smithsetkeywords: + patch, needs review
2010-02-16 00:44:31wdebruijcreate