Patchwork [1/1] sanity.bbclass: check TMPDIR is not too long

login
register
mail settings
Submitter Robert Yang
Date May 8, 2012, 2:50 a.m.
Message ID <228b3adfffa660477cc65e00fc32c95f2bf976fd.1336445265.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/27267/
State New
Headers show

Comments

Robert Yang - May 8, 2012, 2:50 a.m.
When the length of TMPDIR is longer than a threshold, there would be an
"Argument list too long" error when building gcc-cross, this is the
error from the exec(), the maximum length of argument is defined in
/usr/include/linux/limits.h:

  #define ARG_MAX       131072    /* # bytes of args + environ for exec() */

It's hard to determine the threshold of the TMPDIR, here is the
experimental value:
len(TMPDIR) = 182	Success
len(TMPDIR) = 192	Failed

So set the maximum length of TMPDIR to 180 seems proper.

[YOCTO #2434]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes/sanity.bbclass |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Robert Yang - May 8, 2012, 3 a.m.
On 05/08/2012 10:50 AM, Robert Yang wrote:
> When the length of TMPDIR is longer than a threshold, there would be an
> "Argument list too long" error when building gcc-cross, this is the
> error from the exec(), the maximum length of argument is defined in
> /usr/include/linux/limits.h:
>
>    #define ARG_MAX       131072    /* # bytes of args + environ for exec() */
>
> It's hard to determine the threshold of the TMPDIR, here is the
> experimental value:
> len(TMPDIR) = 182	Success
> len(TMPDIR) = 192	Failed
>
> So set the maximum length of TMPDIR to 180 seems proper.
>
> [YOCTO #2434]
>
> Signed-off-by: Robert Yang<liezhi.yang@windriver.com>
> ---
>   meta/classes/sanity.bbclass |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 687ddeb..6aaac1e 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -30,6 +30,7 @@ def check_sanity_sstate_dir_change(sstate_dir, data):
>       testmsg = ""
>       if sstate_dir != "":
>           testmsg = check_create_long_filename(sstate_dir, "SSTATE_DIR")
> +

Sorry, here has a unexpected blank line, I've removed it and pushed to:

git://git.pokylinux.org/poky-contrib robert/tmpdir

// Robert


>       return testmsg
>
>   def check_sanity_tmpdir_change(tmpdir, data):
> @@ -83,6 +84,12 @@ def check_create_long_filename(filepath, pathname):
>               return "Failed to create a file in %s: %s" % (pathname, strerror)
>       return ""
>
> +def check_long_path(filepath, pathname):
> +    # The 180 is just an empirical value
> +    if len(filepath)>  180:
> +	return "The path length of %s is too long, this would cause the \"Argument list too long\" error, please use a shorter path.\n" % pathname
> +    return ""
> +
>   def check_connectivity(d):
>       # URI's to check can be set in the CONNECTIVITY_CHECK_URIS variable
>       # using the same syntax as for SRC_URI. If the variable is not set
> @@ -353,6 +360,10 @@ def check_sanity(e):
>       tmpdir = data.getVar('TMPDIR', e.data, True)
>       sstate_dir = data.getVar('SSTATE_DIR', e.data, True)
>
> +    # Check whether the length of TMPDIR is too long to cause the
> +    # argument list too long error
> +    messages = messages + check_long_path(tmpdir, "TMPDIR")
> +
>       # Check saved sanity info
>       last_sanity_version = 0
>       last_tmpdir = ""
Phil Blundell - May 8, 2012, 7:14 a.m.
On Tue, 2012-05-08 at 10:50 +0800, Robert Yang wrote:
> When the length of TMPDIR is longer than a threshold, there would be an
> "Argument list too long" error when building gcc-cross, this is the
> error from the exec(), the maximum length of argument is defined in
> /usr/include/linux/limits.h:
> 
>   #define ARG_MAX       131072    /* # bytes of args + environ for exec() */
> 
> It's hard to determine the threshold of the TMPDIR, here is the
> experimental value:
> len(TMPDIR) = 182	Success
> len(TMPDIR) = 192	Failed
> 
> So set the maximum length of TMPDIR to 180 seems proper.

It seems a bit lame for paths to be restricted to such a short length.
How does a 192-byte TMPDIR end up causing more than 131072 bytes of
arguments and environment?  Can anything be done to reduce that?  For
example, can you use "gcc @..." to remove common options from the
command line?  Can you eliminate garbage from the environment that
doesn't need to be there?

p.
Robert Yang - May 8, 2012, 9:58 a.m.
Hi Phil,

Thanks for your reply, please see my comments inline ...

On 05/08/2012 03:14 PM, Phil Blundell wrote:
> On Tue, 2012-05-08 at 10:50 +0800, Robert Yang wrote:
>> When the length of TMPDIR is longer than a threshold, there would be an
>> "Argument list too long" error when building gcc-cross, this is the
>> error from the exec(), the maximum length of argument is defined in
>> /usr/include/linux/limits.h:
>>
>>    #define ARG_MAX       131072    /* # bytes of args + environ for exec() */
>>
>> It's hard to determine the threshold of the TMPDIR, here is the
>> experimental value:
>> len(TMPDIR) = 182	Success
>> len(TMPDIR) = 192	Failed
>>
>> So set the maximum length of TMPDIR to 180 seems proper.
>
> It seems a bit lame for paths to be restricted to such a short length.
> How does a 192-byte TMPDIR end up causing more than 131072 bytes of
> arguments and environment?  Can anything be done to reduce that?  For
> example, can you use "gcc @..." to remove common options from the

It doesn't error at the compile stage, but at the install stage, this is
caused by:

gcc-cross-4.6.3+svnr184847-r24/gcc-4_6-branch/build.x86_64-linux.arm-poky-linux-gnueabi/gcc/Makefile:

install-plugin: installdirs lang.install-plugin s-header-vars
# We keep the directory structure for files in config and .def files. All
# other files are flattened to a single directory.
         $(mkinstalldirs) $(DESTDIR)$(plugin_includedir)
         headers=`echo $(PLUGIN_HEADERS) | tr ' ' '\012' | sort -u`; \
         srcdirstrip=`echo "$(srcdir)" | sed 's/[].[^$$\\*|]/\\\\&/g'`; \
         for file in $$headers; do \
	...

The error will happen at the "for" loop when there are many files in $headers,
we can fix the Makefile, but if we don't limit the length of TMPDIR, the similar
error would come out sooner or later since other pkgs may have the similar
issue, so I think that limit the length of TMPDIR may be a better choice.
It seems that seldom people will use a TMPDIR longer than 180 characters,
this is just a prevention.

// Robert

> command line?  Can you eliminate garbage from the environment that
> doesn't need to be there?
>
> p.
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
Robert Yang - May 22, 2012, 10:27 a.m.
I've rebased the code to the up-to-date master branch, and pushed to:

   git://git.pokylinux.org/poky-contrib robert/tmpdir
   http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=robert/tmpdir

// Robert

On 05/08/2012 10:50 AM, Robert Yang wrote:
> When the length of TMPDIR is longer than a threshold, there would be an
> "Argument list too long" error when building gcc-cross, this is the
> error from the exec(), the maximum length of argument is defined in
> /usr/include/linux/limits.h:
>
>    #define ARG_MAX       131072    /* # bytes of args + environ for exec() */
>
> It's hard to determine the threshold of the TMPDIR, here is the
> experimental value:
> len(TMPDIR) = 182	Success
> len(TMPDIR) = 192	Failed
>
> So set the maximum length of TMPDIR to 180 seems proper.
>
> [YOCTO #2434]
>
> Signed-off-by: Robert Yang<liezhi.yang@windriver.com>
> ---
>   meta/classes/sanity.bbclass |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
> index 687ddeb..6aaac1e 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -30,6 +30,7 @@ def check_sanity_sstate_dir_change(sstate_dir, data):
>       testmsg = ""
>       if sstate_dir != "":
>           testmsg = check_create_long_filename(sstate_dir, "SSTATE_DIR")
> +
>       return testmsg
>
>   def check_sanity_tmpdir_change(tmpdir, data):
> @@ -83,6 +84,12 @@ def check_create_long_filename(filepath, pathname):
>               return "Failed to create a file in %s: %s" % (pathname, strerror)
>       return ""
>
> +def check_long_path(filepath, pathname):
> +    # The 180 is just an empirical value
> +    if len(filepath)>  180:
> +	return "The path length of %s is too long, this would cause the \"Argument list too long\" error, please use a shorter path.\n" % pathname
> +    return ""
> +
>   def check_connectivity(d):
>       # URI's to check can be set in the CONNECTIVITY_CHECK_URIS variable
>       # using the same syntax as for SRC_URI. If the variable is not set
> @@ -353,6 +360,10 @@ def check_sanity(e):
>       tmpdir = data.getVar('TMPDIR', e.data, True)
>       sstate_dir = data.getVar('SSTATE_DIR', e.data, True)
>
> +    # Check whether the length of TMPDIR is too long to cause the
> +    # argument list too long error
> +    messages = messages + check_long_path(tmpdir, "TMPDIR")
> +
>       # Check saved sanity info
>       last_sanity_version = 0
>       last_tmpdir = ""

Patch

diff --git a/meta/classes/sanity.bbclass b/meta/classes/sanity.bbclass
index 687ddeb..6aaac1e 100644
--- a/meta/classes/sanity.bbclass
+++ b/meta/classes/sanity.bbclass
@@ -30,6 +30,7 @@  def check_sanity_sstate_dir_change(sstate_dir, data):
     testmsg = ""
     if sstate_dir != "":
         testmsg = check_create_long_filename(sstate_dir, "SSTATE_DIR")
+
     return testmsg
 
 def check_sanity_tmpdir_change(tmpdir, data):
@@ -83,6 +84,12 @@  def check_create_long_filename(filepath, pathname):
             return "Failed to create a file in %s: %s" % (pathname, strerror)
     return ""
 
+def check_long_path(filepath, pathname):
+    # The 180 is just an empirical value
+    if len(filepath) > 180:
+	return "The path length of %s is too long, this would cause the \"Argument list too long\" error, please use a shorter path.\n" % pathname
+    return ""
+
 def check_connectivity(d):
     # URI's to check can be set in the CONNECTIVITY_CHECK_URIS variable
     # using the same syntax as for SRC_URI. If the variable is not set
@@ -353,6 +360,10 @@  def check_sanity(e):
     tmpdir = data.getVar('TMPDIR', e.data, True)
     sstate_dir = data.getVar('SSTATE_DIR', e.data, True)
 
+    # Check whether the length of TMPDIR is too long to cause the
+    # argument list too long error
+    messages = messages + check_long_path(tmpdir, "TMPDIR")
+
     # Check saved sanity info
     last_sanity_version = 0
     last_tmpdir = ""