[PATCH] license: Improve disk usage

[PATCH] license: Improve disk usage

[Richard Purdie]

Currently copies of the license files are made which wastes disk space
and adversely affects performance. We can link these instead in most
cases for small performance gains.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index 601f561..a34ea39 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -145,7 +145,14 @@ def copy_license_files(lic_files_paths, destdir):
     bb.utils.mkdirhier(destdir)
     for (basename, path) in lic_files_paths:
         try:
-            ret = shutil.copyfile(path, os.path.join(destdir, basename))
+            src = path
+            dst = os.path.join(destdir, basename)
+            if os.path.exists(dst):
+                os.remove(dst)
+            if (os.stat(src).st_dev == os.stat(destdir).st_dev):
+                os.link(src, dst)
+            else:
+                shutil.copyfile(src, dst)
         except Exception as e:
             bb.warn("Could not copy license file %s: %s" % (basename, e))
 

Re: [PATCH] license: Improve disk usage

[Mike Looijmans]

I can probably come up with scenarios where "os.stat(src).st_dev == 
os.stat(destdir).st_dev" but os.link will still fail.

Less code and does not assume that when src and destdir are on the same 
device, they can be linked:

try:
   os.link(src, dst)
except:
   shutil.copyfile(src, dst)

Mike.


On 09/12/2014 05:39 PM, Richard Purdie wrote:
> Currently copies of the license files are made which wastes disk space
> and adversely affects performance. We can link these instead in most
> cases for small performance gains.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index 601f561..a34ea39 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -145,7 +145,14 @@ def copy_license_files(lic_files_paths, destdir):
>       bb.utils.mkdirhier(destdir)
>       for (basename, path) in lic_files_paths:
>           try:
> -            ret = shutil.copyfile(path, os.path.join(destdir, basename))
> +            src = path
> +            dst = os.path.join(destdir, basename)
> +            if os.path.exists(dst):
> +                os.remove(dst)
> +            if (os.stat(src).st_dev == os.stat(destdir).st_dev):
> +                os.link(src, dst)
> +            else:
> +                shutil.copyfile(src, dst)
>           except Exception as e:
>               bb.warn("Could not copy license file %s: %s" % (basename, e))
>
>
>



Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Topic zoekt gedreven (embedded) software specialisten!
http://topic.nl/vacatures/topic-zoekt-software-engineers/

Re: [PATCH] license: Improve disk usage

[Richard Purdie]

On Mon, 2014-09-15 at 15:58 +0200, Mike Looijmans wrote:
> I can probably come up with scenarios where "os.stat(src).st_dev == 
> os.stat(destdir).st_dev" but os.link will still fail.
> 
> Less code and does not assume that when src and destdir are on the same 
> device, they can be linked:
> 
> try:
>    os.link(src, dst)
> except:
>    shutil.copyfile(src, dst)

I agree that is probably easier. I'm curious which scenarios you're
thinking of though. We do have that device comparison elsewhere in the
codebase and I copied it mainly as it was how it was done elsewhere...

Cheers,

Richard

Re: [PATCH] license: Improve disk usage

[Mike Looijmans]

On 09/15/2014 04:12 PM, Richard Purdie wrote:
> On Mon, 2014-09-15 at 15:58 +0200, Mike Looijmans wrote:
>> I can probably come up with scenarios where "os.stat(src).st_dev ==
>> os.stat(destdir).st_dev" but os.link will still fail.
>>
>> Less code and does not assume that when src and destdir are on the same
>> device, they can be linked:
>>
>> try:
>>     os.link(src, dst)
>> except:
>>     shutil.copyfile(src, dst)
>
> I agree that is probably easier. I'm curious which scenarios you're
> thinking of though. We do have that device comparison elsewhere in the
> codebase and I copied it mainly as it was how it was done elsewhere...
>

Simplest case is when the underlying filesystem does not support hardlinks.

I admit that chances of that being the case here will be very very small.

It's just that the try/except version will always work on any system, whereas 
the device comparison may fail on some setups. And given the habit of 
programmers to copy these things from other locations, chances are that a 
device comparison will some day be used on the "downloads" directory which is 
FAT on my system.

Also, when the copy fails, the resulting message should be clearer, as the 
"copyfile" call will be in the stacktrace.

Mike.


Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Topic zoekt gedreven (embedded) software specialisten!
http://topic.nl/vacatures/topic-zoekt-software-engineers/

Re: [PATCH] license: Improve disk usage

[Burton, Ross]

On 15 September 2014 15:47, Mike Looijmans <mike.looijmans@topic.nl> wrote:
> It's just that the try/except version will always work on any system,
> whereas the device comparison may fail on some setups. And given the habit
> of programmers to copy these things from other locations, chances are that a
> device comparison will some day be used on the "downloads" directory which
> is FAT on my system.
>
> Also, when the copy fails, the resulting message should be clearer, as the
> "copyfile" call will be in the stacktrace.

I believe this is called the "try and ask forgiveness" pattern.  It's
race-free, too.

Ross