Patchwork SDK: trap any IO errors in the relocate script

login
register
mail settings
Submitter Laurentiu Palcu
Date Sept. 25, 2012, 4:35 p.m.
Message ID <1348590946-13319-1-git-send-email-laurentiu.palcu@intel.com>
Download mbox | patch
Permalink /patch/37209/
State Accepted
Commit 26daec758b2eaeb208356d5aa8a9a191bd366751
Headers show

Comments

Laurentiu Palcu - Sept. 25, 2012, 4:35 p.m.
If the files being relocated are already used by other processes the
relocate script will fail with a traceback. This patch will trap any IO
errors when opening such a file and gracefully report them to the user.

Also change the exit code from 1 to -1 for a better adt-installer user
experience (like pointing the user to the adt_installer.log).

[YOCTO #3164]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 scripts/relocate_sdk.py |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
Saul Wold - Sept. 27, 2012, 3:49 p.m.
On 09/25/2012 09:35 AM, Laurentiu Palcu wrote:

BTW: in the future it's good to say the "filename: <commit subject>"

So this would be "relocate_sdk.py: ...."
> If the files being relocated are already used by other processes the
> relocate script will fail with a traceback. This patch will trap any IO
> errors when opening such a file and gracefully report them to the user.
>
> Also change the exit code from 1 to -1 for a better adt-installer user
> experience (like pointing the user to the adt_installer.log).
>
> [YOCTO #3164]
>
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>   scripts/relocate_sdk.py |   14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/relocate_sdk.py b/scripts/relocate_sdk.py
> index b247e65..637ffe9 100755
> --- a/scripts/relocate_sdk.py
> +++ b/scripts/relocate_sdk.py
> @@ -29,6 +29,7 @@ import sys
>   import stat
>   import os
>   import re
> +import errno
>
>   old_prefix = re.compile("##DEFAULT_INSTALL_DIR##")
>
> @@ -171,7 +172,7 @@ def change_dl_sysdirs():
>
>   # MAIN
>   if len(sys.argv) < 4:
> -    exit(1)
> +    exit(-1)
>
>   new_prefix = sys.argv[1]
>   new_dl_path = sys.argv[2]
> @@ -184,7 +185,16 @@ for e in executables_list:
>       else:
>           os.chmod(e, perms|stat.S_IRWXU)
>
> -    f = open(e, "r+b")
> +    try:
> +        f = open(e, "r+b")
> +    except IOError as ioex:
> +        if ioex.errno == errno.ETXTBSY:
> +            print("Could not open %s. File used by another process.\nPlease "\
> +                  "make sure you exit all processes that might use any SDK "\
> +                  "binaries." % e)
> +        else:
> +            print("Could not open %s: %s(%d)" % (e, ioex.strerror, ioex.errno))
> +        exit(-1)
>
>       arch = get_arch()
>       if arch:
>
Merged into OE-Core

Thanks
	Sau!
Laurentiu Palcu - Sept. 28, 2012, 6:50 a.m.
On 09/27/2012 06:49 PM, Saul Wold wrote:
> On 09/25/2012 09:35 AM, Laurentiu Palcu wrote:
> 
> BTW: in the future it's good to say the "filename: <commit subject>"
This is what I usually use when I change single files. However,
sometimes (and I know this particular patch is not the case), I change
multiple files to fix a single problem or add a feature. In this case
your BKM is not quite appropriate. I will end up with a commit subject
like this:

filename1, filename2, filename3: <no space left for the subject itself>

If those files belong to a certain functional area, say SDK, I thought
it was more appropriate to prepend the subject with SDK.

Also, the contribution guidelines on the wiki, state the same (see rpm
example):

https://wiki.yoctoproject.org/wiki/Contribution_Guidelines

Am I wrong here?

Thanks,
Laurentiu

> 
> So this would be "relocate_sdk.py: ...."
>> If the files being relocated are already used by other processes the
>> relocate script will fail with a traceback. This patch will trap any IO
>> errors when opening such a file and gracefully report them to the user.
>>
>> Also change the exit code from 1 to -1 for a better adt-installer user
>> experience (like pointing the user to the adt_installer.log).
>>
>> [YOCTO #3164]
>>
>> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
>> ---
>>   scripts/relocate_sdk.py |   14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/relocate_sdk.py b/scripts/relocate_sdk.py
>> index b247e65..637ffe9 100755
>> --- a/scripts/relocate_sdk.py
>> +++ b/scripts/relocate_sdk.py
>> @@ -29,6 +29,7 @@ import sys
>>   import stat
>>   import os
>>   import re
>> +import errno
>>
>>   old_prefix = re.compile("##DEFAULT_INSTALL_DIR##")
>>
>> @@ -171,7 +172,7 @@ def change_dl_sysdirs():
>>
>>   # MAIN
>>   if len(sys.argv) < 4:
>> -    exit(1)
>> +    exit(-1)
>>
>>   new_prefix = sys.argv[1]
>>   new_dl_path = sys.argv[2]
>> @@ -184,7 +185,16 @@ for e in executables_list:
>>       else:
>>           os.chmod(e, perms|stat.S_IRWXU)
>>
>> -    f = open(e, "r+b")
>> +    try:
>> +        f = open(e, "r+b")
>> +    except IOError as ioex:
>> +        if ioex.errno == errno.ETXTBSY:
>> +            print("Could not open %s. File used by another process.\nPlease "\
>> +                  "make sure you exit all processes that might use any SDK "\
>> +                  "binaries." % e)
>> +        else:
>> +            print("Could not open %s: %s(%d)" % (e, ioex.strerror, ioex.errno))
>> +        exit(-1)
>>
>>       arch = get_arch()
>>       if arch:
>>
> Merged into OE-Core
> 
> Thanks
> 	Sau!
>
Richard Purdie - Sept. 28, 2012, 3:46 p.m.
On Fri, 2012-09-28 at 09:50 +0300, Laurentiu Palcu wrote:
> 
> On 09/27/2012 06:49 PM, Saul Wold wrote:
> > On 09/25/2012 09:35 AM, Laurentiu Palcu wrote:
> > 
> > BTW: in the future it's good to say the "filename: <commit subject>"
> This is what I usually use when I change single files. However,
> sometimes (and I know this particular patch is not the case), I change
> multiple files to fix a single problem or add a feature. In this case
> your BKM is not quite appropriate. I will end up with a commit subject
> like this:
> 
> filename1, filename2, filename3: <no space left for the subject itself>
> 
> If those files belong to a certain functional area, say SDK, I thought
> it was more appropriate to prepend the subject with SDK.
> 
> Also, the contribution guidelines on the wiki, state the same (see rpm
> example):
> 
> https://wiki.yoctoproject.org/wiki/Contribution_Guidelines
> 
> Am I wrong here?

No but I think "SDK" is just a little unspecific so for example a hint
that this was a change to "scripts:"  would have been more useful which
I think was Saul's point.

Cheers,

Richard

Patch

diff --git a/scripts/relocate_sdk.py b/scripts/relocate_sdk.py
index b247e65..637ffe9 100755
--- a/scripts/relocate_sdk.py
+++ b/scripts/relocate_sdk.py
@@ -29,6 +29,7 @@  import sys
 import stat
 import os
 import re
+import errno
 
 old_prefix = re.compile("##DEFAULT_INSTALL_DIR##")
 
@@ -171,7 +172,7 @@  def change_dl_sysdirs():
 
 # MAIN
 if len(sys.argv) < 4:
-    exit(1)
+    exit(-1)
 
 new_prefix = sys.argv[1]
 new_dl_path = sys.argv[2]
@@ -184,7 +185,16 @@  for e in executables_list:
     else:
         os.chmod(e, perms|stat.S_IRWXU)
 
-    f = open(e, "r+b")
+    try:
+        f = open(e, "r+b")
+    except IOError as ioex:
+        if ioex.errno == errno.ETXTBSY:
+            print("Could not open %s. File used by another process.\nPlease "\
+                  "make sure you exit all processes that might use any SDK "\
+                  "binaries." % e)
+        else:
+            print("Could not open %s: %s(%d)" % (e, ioex.strerror, ioex.errno))
+        exit(-1)
 
     arch = get_arch()
     if arch: