bitbake: Correct broken symlink behaviour
PATCH 1/1 bitbake: Correct broken symlink behaviour
From: Roman Savchenko <[email protected]>
Unlink broken symlink.
--- bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
bitbake/lib/bb/fetch2/__init__.py +13 -8
@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath) +def try_symlink(source, link_name): + """ + Tries to make link to passed source if link name does not + exist. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) + def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url
@@ -962,20 +973,14 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): bb.utils.mkdirhier(os.path.dirname(ud.donestamp)) open(ud.donestamp, 'w').close() dest = os.path.join(dldir, os.path.basename(ud.localpath)) - if not os.path.exists(dest): - os.symlink(ud.localpath, dest) + try_symlink(ud.localpath, dest) if not verify_donestamp(origud, ld) or origud.method.need_update(origud, ld): origud.method.download(origud, ld) if hasattr(origud.method,"build_mirror_data"): origud.method.build_mirror_data(origud, ld) return origud.localpath # Otherwise the result is a local file:// and we symlink to it - if not os.path.exists(origud.localpath): - if os.path.islink(origud.localpath): - # Broken symbolic link - os.unlink(origud.localpath) - - os.symlink(ud.localpath, origud.localpath) + try_symlink(ud.localpath, origud.localpath) update_stamp(origud, ld) return ud.localpath
Signature · 1 line
2.7.4
Hi,
Quoted 3 lines · Roman Savchenko
From: Roman Savchenko <[email protected]>
Unlink broken symlink.
This really needs a few more details about the problem and how an issue occurs. I can understand from reading the patch but not from the commit message.
Quoted 15 lines
Signed-off-by Roman Savchenko <[email protected]>---
bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++--------1 file changed, 13 insertions(+), 8 deletions(-)bitbake/lib/bb/fetch2/__init__.py +0
-0@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath)
bitbake/lib/bb/fetch2/__init__.py +1
-0+def try_symlink(source, link_name):
bitbake/lib/bb/fetch2/__init__.py def rename_bad_checksum(ud, suffix): · line 938
Quoted 10 lines
bitbake/lib/bb/fetch2/__init__.py +10
-0+ """ + Tries to make link to passed source if link name does not + exist. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) +
I don't think the function name is right. "try" implies its fine if it doesn't work out. In reality its more like "symlink if its invalid". "create_valid_symlink" whilst long is probably more reflective of what it does but I'm sure we could still do better :/.
You could add "or the symlink is invalid" to the function description too.
Cheers,
Richard
PATCH 1/1 bitbake: Correct broken symlink behaviour
In reply to Roman SavchenkoFrom: Roman Savchenko <[email protected]>
When making new symlink in case when link name does not exist possible broken symlink behaviour should be taken into account.
--- bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
bitbake/lib/bb/fetch2/__init__.py +13 -8
@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath) +def try_symlink(source, link_name): + """ + Tries to make link to passed source if link name does not + exist or it is the invalid symlink. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) + def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url
@@ -962,20 +973,14 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): bb.utils.mkdirhier(os.path.dirname(ud.donestamp)) open(ud.donestamp, 'w').close() dest = os.path.join(dldir, os.path.basename(ud.localpath)) - if not os.path.exists(dest): - os.symlink(ud.localpath, dest) + try_symlink(ud.localpath, dest) if not verify_donestamp(origud, ld) or origud.method.need_update(origud, ld): origud.method.download(origud, ld) if hasattr(origud.method,"build_mirror_data"): origud.method.build_mirror_data(origud, ld) return origud.localpath # Otherwise the result is a local file:// and we symlink to it - if not os.path.exists(origud.localpath): - if os.path.islink(origud.localpath): - # Broken symbolic link - os.unlink(origud.localpath) - - os.symlink(ud.localpath, origud.localpath) + try_symlink(ud.localpath, origud.localpath) update_stamp(origud, ld) return ud.localpath
Signature · 1 line
2.7.4
Hi Richard,
I adjusted commit message and function description. but I do not agree with proposed
function name create_valid_symlink, becasue link creation could fail - so I left try_symlink
to point this behaviour.
Regards,
Roman
2016-09-23 10:45 GMT+03:00 Roman Savchenko <[email protected]>:
Quoted 62 lines
From: Roman Savchenko <[email protected]>
When making new symlink in case when link name does not exist possible broken symlink behaviour should be taken into account.
Signed-off-by Roman Savchenko <[email protected]>--- bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)bitbake/lib/bb/fetch2/__ renamed from bitbake/lib/bb/fetch2/__init__.py +0
-0
init__.py
index 6ef0c6f..d8a3b96 100644bitbake/lib/bb/fetch2/__init__.py +11
-0@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath) +def try_symlink(source, link_name): + """ + Tries to make link to passed source if link name does not + exist or it is the invalid symlink. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) + def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url@@ -962,20 +973,14 @@ def try_mirror_url(fetch, origud, ud, ld, check =False):
bb.utils.mkdirhier(os.path.dirname(ud.donestamp))open(ud.donestamp, 'w').close()
dest =os.path.join(dldir, os.path.basename(ud.localpath))bitbake/lib/bb/fetch2/__init__.py +1
-2- if not os.path.exists(dest): - os.symlink(ud.localpath, dest) + try_symlink(ud.localpath, dest) if not verify_donestamp(origud, ld) or
origud.method.need_update(origud, ld):
origud.method.download(origud, ld)
ifhasattr(origud.method,"build_mirror_data"):
origud.method.build_mirror_data(origud, ld)
return origud.localpath
# Otherwise the result is a local file:// and we symlink to itbitbake/lib/bb/fetch2/__init__.py +1
-7- if not os.path.exists(origud.localpath): - if os.path.islink(origud.localpath): - # Broken symbolic link - os.unlink(origud.localpath) - - os.symlink(ud.localpath, origud.localpath) + try_symlink(ud.localpath, origud.localpath) update_stamp(origud, ld) return ud.localpath --2.7.4
Hi Roman,
Quoted 5 lines · Roman Savchenko
I adjusted commit message and function description. but I do not agree with proposed
function namecreate_valid_symlink, becasue link creation could fail - so I lefttry_symlink
to point this behaviour.
I don't agree with "try_symlink" though, sorry.
Link creation can fail but that is like any other function which can also fail. We don't prefix all function names with try_ because they can fail.
We need a better name.
Cheers,
Richard
PATCH 1/1 bitbake: Correct broken symlink behaviour
In reply to Roman SavchenkoFrom: Roman Savchenko <[email protected]>
When making new symlink in case when link name does not exist possible broken symlink behaviour should be taken into account.
--- bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
bitbake/lib/bb/fetch2/__init__.py +13 -8
@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath) +def create_symlink_if_not_exist(source, link_name): + """ + Tries to make link to passed source if link name does not + exist or it is the invalid symlink. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) + def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url
@@ -962,20 +973,14 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): bb.utils.mkdirhier(os.path.dirname(ud.donestamp)) open(ud.donestamp, 'w').close() dest = os.path.join(dldir, os.path.basename(ud.localpath)) - if not os.path.exists(dest): - os.symlink(ud.localpath, dest) + create_symlink_if_not_exist(ud.localpath, dest) if not verify_donestamp(origud, ld) or origud.method.need_update(origud, ld): origud.method.download(origud, ld) if hasattr(origud.method,"build_mirror_data"): origud.method.build_mirror_data(origud, ld) return origud.localpath # Otherwise the result is a local file:// and we symlink to it - if not os.path.exists(origud.localpath): - if os.path.islink(origud.localpath): - # Broken symbolic link - os.unlink(origud.localpath) - - os.symlink(ud.localpath, origud.localpath) + create_symlink_if_not_exist(ud.localpath, origud.localpath) update_stamp(origud, ld) return ud.localpath
Signature · 1 line
2.7.4
Hi Richard,
I renamed function, probably name is a little bit long, but it fully displays what it does.
Regards,
Roman.
2016-09-27 13:22 GMT+03:00 Roman Savchenko <[email protected]>:
Quoted 62 lines
From: Roman Savchenko <[email protected]>
When making new symlink in case when link name does not exist possible broken symlink behaviour should be taken into account.
Signed-off-by Roman Savchenko <[email protected]>--- bitbake/lib/bb/fetch2/__init__.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)bitbake/lib/bb/fetch2/__ renamed from bitbake/lib/bb/fetch2/__init__.py +0
-0
init__.py
index 6ef0c6f..6638b34 100644bitbake/lib/bb/fetch2/__init__.py +11
-0@@ -925,6 +925,17 @@ def rename_bad_checksum(ud, suffix): bb.utils.movefile(ud.localpath, new_localpath) +def create_symlink_if_not_exist(source, link_name): + """ + Tries to make link to passed source if link name does not + exist or it is the invalid symlink. + """ + if not os.path.exists(link_name): + if os.path.islink(link_name): + os.unlink(link_name) + + os.symlink(source, link_name) + def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url@@ -962,20 +973,14 @@ def try_mirror_url(fetch, origud, ud, ld, check =False):
bb.utils.mkdirhier(os.path.dirname(ud.donestamp))open(ud.donestamp, 'w').close()
dest =os.path.join(dldir, os.path.basename(ud.localpath))bitbake/lib/bb/fetch2/__init__.py +1
-2- if not os.path.exists(dest): - os.symlink(ud.localpath, dest) + create_symlink_if_not_exist(ud.localpath, dest) if not verify_donestamp(origud, ld) or
origud.method.need_update(origud, ld):
origud.method.download(origud, ld)
ifhasattr(origud.method,"build_mirror_data"):
origud.method.build_mirror_data(origud, ld)
return origud.localpath
# Otherwise the result is a local file:// and we symlink to itbitbake/lib/bb/fetch2/__init__.py +1
-7- if not os.path.exists(origud.localpath): - if os.path.islink(origud.localpath): - # Broken symbolic link - os.unlink(origud.localpath) - - os.symlink(ud.localpath, origud.localpath) + create_symlink_if_not_exist(ud.localpath, origud.localpath) update_stamp(origud, ld) return ud.localpath --2.7.4