PLEASE READ: Major change landing shortly (python whitespace)

* PLEASE READ: Major change landing shortly (python whitespace)
@ 2012-07-18 10:06 Richard Purdie
  2012-07-18 10:17   ` Burton, Ross
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Richard Purdie @ 2012-07-18 10:06 UTC (permalink / raw)
  To: openembedded-core; +Cc: bitbake-devel

It's become clear we have a horrible mixture of whitespace (tabs and
space) in python functions. At first glance this sounds trivial but
there is a serious problem if pieces of code get mixed together with
different whitespace since they may or may not work as intended.

The documented standard for python functions is four space indentation
although we have a mixture. Fixing this sounds simple at first, we just
go through and change tabs to spaces. The problem comes with _append and
_prepend to python functions since both need their whitespace changed at
the same time. populate_packages() is the real problem child in that
regard but its the less common examples I worry about.

I did some research and we have inconsistencies, even in existing
functions since people are editing files and getting confused
(understandably).

So the question becomes, how do we get ourselves out of this mess?

I put a proposal to the TSC, that we have bitbake warn/error whenever it
finds tab characters in any python function. The advantage of this is
that we give the user a clear definitive error. The downside is that
we'll have to go through all the metadata and scrub it for the problem.

The TSC agrees we should do something about this rather than perpetuate
the problem and that this is as good a solution as any of us can come up
with. We also agreed that we should do it sooner than later before it
gets any later in this release cycle. Putting it off until the next
release cycle didn't seem to offer any advantage, we might as well get
on with it.

So I am going to:

a) Try and flush through as many pending patches as I can.
b) Check in a warning into bitbake master and increase its version
c) Require that version in OE-Core
d) Commit a significant set of whitespace changes to OE-Core, resolving
all the warnings for OE-Core.

I plan to do this, tomorrow, Thursday.

The reason for getting on with it is that the patches are horrible to
carry around and that any other patches made against OE-Core will likely
need to be remade to apply after these changes. I therefore don't want a
week of waiting around discussing it, this is something I believe we
need to do now and be done with it. I'm also not going to post the
whitespace patches for review, I'll write+merge them and then be
responsive for accepting fixes for anything that gets broken. They will
only be whitespace changes.

I appreciate this is going to cause some disruption but I think there is
a problem worth solving here and this is the best way to do it. If
anyone has any strong objections I'm open to alternative ideas.

The bitbake patch to error on tab whitespace is included below. I'll try
and post a branch with the whitespace fixes on later today.

Cheers,

Richard

diff --git a/bitbake/lib/bb/data.py b/bitbake/lib/bb/data.py
index e3ffefe..4b653a5 100644
--- a/bitbake/lib/bb/data.py
+++ b/bitbake/lib/bb/data.py
@@ -291,6 +291,8 @@ def build_dependencies(key, keys, shelldeps, vardepvals, d):
             if d.getVarFlag(key, "python"):
                 parsedvar = d.expandWithRefs(value, key)
                 parser = bb.codeparser.PythonParser(key, logger)
+                if parsedvar.value and "\t" in parsedvar.value:
+                    bb.warn("Variable %s contains tabs, please remove these" % key)
                 parser.parse_python(parsedvar.value)
                 deps = deps | parser.references
             else:
diff --git a/bitbake/lib/bb/parse/ast.py b/bitbake/lib/bb/parse/ast.py
index eae840f..86f9463 100644
--- a/bitbake/lib/bb/parse/ast.py
+++ b/bitbake/lib/bb/parse/ast.py
@@ -212,9 +212,9 @@ class ExportFuncsNode(AstNode):
                         data.setVarFlag(calledvar, flag, data.getVarFlag(var, flag))
 
                 if data.getVarFlag(calledvar, "python"):
-                    data.setVar(var, "\tbb.build.exec_func('" + calledvar + "', d)\n")
+                    data.setVar(var, "    bb.build.exec_func('" + calledvar + "', d)\n")
                 else:
-                    data.setVar(var, "\t" + calledvar + "\n")
+                    data.setVar(var, "    " + calledvar + "\n")
                 data.setVarFlag(var, 'export_func', '1')
 
 class AddTaskNode(AstNode):





^ permalink raw reply related	[flat|nested] 24+ messages in thread