Closed Bug 389393 Opened 15 years ago Closed 10 years ago

rewrite Makefile generation logic for configure rewrite


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: ted, Unassigned)





(1 file, 1 obsolete file)

Attached file config/ (obsolete) —
I have a mostly working reimplementation of  We'll probably want to extend it so it can handle being an AC_OUTPUT replacement as well, but that shouldn't be a huge effort.  The only real difference between my implementation and the is that my impl puts in full pathnames for srcdir, top_srcdir, whereas puts in weird relative paths.  I don't know which is more correct, really.

This is just the Python script, not integrated into the configure process at all  yet.
Assignee: nobody → ted.mielczarek
Target Milestone: --- → mozilla2.0
This code is almost smart enough to replace AC_OUTPUT as well, but I need to slog through some more m4 before I can do that.  I'll probably just add a commandline option to the script to tell it to read replacements from stdin, so then we could run it with a HERE doc with all the replacments.
Attachment #273578 - Attachment is obsolete: true
Attachment #275848 - Flags: review?(benjamin)
Comment on attachment 275848 [details] [diff] [review]
replace in

1) what path style does this end up with on Windows?

2) I don't really understand mkdirs... the docstring says something about returning, but it doesn't return anything, and I think you could probably make good use of os.makedirs
Attachment #275848 - Flags: review?(benjamin) → review-
Comment on attachment 275848 [details] [diff] [review]
replace in

Random uninvited comments.

Is splitPath going to get OS-native paths fed or '/'? I expect the latter from our, right? Might be that path.split('/')[:-1] is what you really want.

>+def splitPath(path):

>+def mkdirs(files, basedir):
What :bs said, though, don't raise "ouch", too.

>+def do_replacement(s, replacements, outfile):
>+  """Given a string |s|, and a dict |replacements| of strings to replace,
>+  replace all occurences of keys in |replacements| with their values,
>+  and write the output to outfile.
>+  """
>+  for (key, value) in replacements.iteritems():
>+    s = s.replace(key, value)
>+  if re.match("@[_A-Za-z]+@", s):
>+    return False
>+  outfile.write(s)
>+  return True

I'd suggest something like

  s = re.sub('"@([_A-Za-z]+)@", lambda m: replacements[], s)
except KeyError:
  return False

and to not include the @@ in the logic below.

>+def substFiles(files, srcDir, objDir, replacements={}):
>+  files.sort()
>+  mkdirs((f for f in files), objDir)
if mkdirs doesn't modify files, you don't need to create a generator here. You could
just do iter(files), too, if you wanted to make sure it's readonly.

>+  unhandledFiles = []
>+  haveReplacements = len(dict) > 0
>+  replacements["@top_srcdir@"] = srcDir,
>+  relPath = True
>+  if srcDir.startswith("/"): # don't know of a better way to check this
>+    relPath = False

how about

  relPath = srcDir != os.path.abspath(srcDir)

>+    if relPath:
>+      # preserve relative paths
>+      prefix = "/".join(".." for x in xrange(len(splitPath(file))))
>+      topSrcDir = os.path.join(prefix, srcDir)

      topSrcDir = os.path.join(*(['..']*len(splitPath(file)) + [srcDir])

would do the trickm, and join the '..' right.

>+      replacements["@top_srcdir@"] = topSrcDir
>+    if subDir == "":
>+      # just to avoid the trailing slash
>+      localSrcDir = topSrcDir
>+    else:
>+      localSrcDir = os.path.join(topSrcDir, subDir)

nit on above,
    if not subDir:

>+    replacements["@srcdir@"] = localSrcDir
>+    inFH = open(fileIn, "r")
>+    outFH = open(fileOut, "w")
>+    try:
>+      if not do_replacement(, replacements, outFH):
>+        unhandledFiles.append(file)
>+    finally:
>+      inFH.close()
>+      outFH.close()

I read complaints about finally and python 2.3 somewhere, does that matter for build?
Assignee: ted.mielczarek → nobody
Closed: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 784841
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.