Re: [Mesa-dev] [PATCH v2] meson, install_megadrivers: Also remove stale symlinks
Am Montag, den 06.08.2018, 16:59 +0100 schrieb Eric Engestrom:
> On Monday, 2018-08-06 09:41:48 +0200, Gert Wollny wrote:
> > os.path.exists doesn't return True for stale symlinks, but they are
> > in
> > the way later, when a link/file with the same name is to be
> > created.
> > For instance it is conceivable that the pointed to file is replaced
> > by
> > a file with a new name, and then the symlink is dead.
> >
> > To handle this check specifically for all existing symlinks to be
> > removed. (This bugged me for some time with a link libXvMCr600.so
> > always being in the way of installing this file)
> >
> > v2: use only os.path.lexist and replace all instances
> > of os.path.exist (Dylan Baker)
> >
> > Fixes: f7f1b30f81e842db6057591470ce3cb6d4fb2795
> > ("meson: extend install_megadrivers script to handle
> > symmlinking")
> >
> > Signed-off-by: Gert Wollny
> > ---
> > bin/install_megadrivers.py | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/bin/install_megadrivers.py
> > b/bin/install_megadrivers.py
> > index 8d9ed9c6dc..3a31fddd89 100755
> > --- a/bin/install_megadrivers.py
> > +++ b/bin/install_megadrivers.py
> > @@ -42,14 +42,14 @@ def main():
> >
> > master = os.path.join(to, os.path.basename(args.megadriver))
> >
> > -if not os.path.exists(to):
> > +if not os.path.lexists(to):
> > os.makedirs(to)
>
> I'm not convinced on this one: if the `to` destination is a symlink
> but the destination of the symlink doesn't exist, we shouldn't just
> consider that the folder has already been created and is ready for
> use.
You're right, I guess it was too early this morning, so that I just
changed all instances of os.exists mechnically,
>
> I think here we should either just ignore that case, or if we want to
> handle it then it should be:
> if not exists(to):
> if lexists(to):
> unlink(to)
> makedirs(to)
Fixes like this locally.
best,
Gert
>
> The other two below are
> Reviewed-by: Eric Engestrom
>
> > shutil.copy(args.megadriver, master)
> >
> > for driver in args.drivers:
> > abs_driver = os.path.join(to, driver)
> >
> > -if os.path.exists(abs_driver):
> > +if os.path.lexists(abs_driver):
> > os.unlink(abs_driver)
> > print('installing {} to {}'.format(args.megadriver,
> > abs_driver))
> > os.link(master, abs_driver)
> > @@ -60,7 +60,7 @@ def main():
> >
> > name, ext = os.path.splitext(driver)
> > while ext != '.so':
> > -if os.path.exists(name):
> > +if os.path.lexists(name):
> > os.unlink(name)
> > os.symlink(driver, name)
> > name, ext = os.path.splitext(name)
> > --
> > 2.16.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] meson, install_megadrivers: Also remove stale symlinks
Quoting Eric Engestrom (2018-08-06 08:59:00)
> On Monday, 2018-08-06 09:41:48 +0200, Gert Wollny wrote:
> > os.path.exists doesn't return True for stale symlinks, but they are in
> > the way later, when a link/file with the same name is to be created.
> > For instance it is conceivable that the pointed to file is replaced by
> > a file with a new name, and then the symlink is dead.
> >
> > To handle this check specifically for all existing symlinks to be
> > removed. (This bugged me for some time with a link libXvMCr600.so
> > always being in the way of installing this file)
> >
> > v2: use only os.path.lexist and replace all instances
> > of os.path.exist (Dylan Baker)
> >
> > Fixes: f7f1b30f81e842db6057591470ce3cb6d4fb2795
> > ("meson: extend install_megadrivers script to handle symmlinking")
> >
> > Signed-off-by: Gert Wollny
> > ---
> > bin/install_megadrivers.py | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/bin/install_megadrivers.py b/bin/install_megadrivers.py
> > index 8d9ed9c6dc..3a31fddd89 100755
> > --- a/bin/install_megadrivers.py
> > +++ b/bin/install_megadrivers.py
> > @@ -42,14 +42,14 @@ def main():
> >
> > master = os.path.join(to, os.path.basename(args.megadriver))
> >
> > -if not os.path.exists(to):
> > +if not os.path.lexists(to):
> > os.makedirs(to)
>
> I'm not convinced on this one: if the `to` destination is a symlink but
> the destination of the symlink doesn't exist, we shouldn't just consider
> that the folder has already been created and is ready for use.
>
> I think here we should either just ignore that case, or if we want to
> handle it then it should be:
> if not exists(to):
> if lexists(to):
> unlink(to)
> makedirs(to)
>
> The other two below are
> Reviewed-by: Eric Engestrom
Good catch, I wasn't paying good enough attention :/
With either Eric's suggestion, or the first hunk removed:
Reviewed-by: Dylan Baker
>
> > shutil.copy(args.megadriver, master)
> >
> > for driver in args.drivers:
> > abs_driver = os.path.join(to, driver)
> >
> > -if os.path.exists(abs_driver):
> > +if os.path.lexists(abs_driver):
> > os.unlink(abs_driver)
> > print('installing {} to {}'.format(args.megadriver, abs_driver))
> > os.link(master, abs_driver)
> > @@ -60,7 +60,7 @@ def main():
> >
> > name, ext = os.path.splitext(driver)
> > while ext != '.so':
> > -if os.path.exists(name):
> > +if os.path.lexists(name):
> > os.unlink(name)
> > os.symlink(driver, name)
> > name, ext = os.path.splitext(name)
> > --
> > 2.16.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] meson, install_megadrivers: Also remove stale symlinks
On Monday, 2018-08-06 09:41:48 +0200, Gert Wollny wrote:
> os.path.exists doesn't return True for stale symlinks, but they are in
> the way later, when a link/file with the same name is to be created.
> For instance it is conceivable that the pointed to file is replaced by
> a file with a new name, and then the symlink is dead.
>
> To handle this check specifically for all existing symlinks to be
> removed. (This bugged me for some time with a link libXvMCr600.so
> always being in the way of installing this file)
>
> v2: use only os.path.lexist and replace all instances
> of os.path.exist (Dylan Baker)
>
> Fixes: f7f1b30f81e842db6057591470ce3cb6d4fb2795
> ("meson: extend install_megadrivers script to handle symmlinking")
>
> Signed-off-by: Gert Wollny
> ---
> bin/install_megadrivers.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bin/install_megadrivers.py b/bin/install_megadrivers.py
> index 8d9ed9c6dc..3a31fddd89 100755
> --- a/bin/install_megadrivers.py
> +++ b/bin/install_megadrivers.py
> @@ -42,14 +42,14 @@ def main():
>
> master = os.path.join(to, os.path.basename(args.megadriver))
>
> -if not os.path.exists(to):
> +if not os.path.lexists(to):
> os.makedirs(to)
I'm not convinced on this one: if the `to` destination is a symlink but
the destination of the symlink doesn't exist, we shouldn't just consider
that the folder has already been created and is ready for use.
I think here we should either just ignore that case, or if we want to
handle it then it should be:
if not exists(to):
if lexists(to):
unlink(to)
makedirs(to)
The other two below are
Reviewed-by: Eric Engestrom
> shutil.copy(args.megadriver, master)
>
> for driver in args.drivers:
> abs_driver = os.path.join(to, driver)
>
> -if os.path.exists(abs_driver):
> +if os.path.lexists(abs_driver):
> os.unlink(abs_driver)
> print('installing {} to {}'.format(args.megadriver, abs_driver))
> os.link(master, abs_driver)
> @@ -60,7 +60,7 @@ def main():
>
> name, ext = os.path.splitext(driver)
> while ext != '.so':
> -if os.path.exists(name):
> +if os.path.lexists(name):
> os.unlink(name)
> os.symlink(driver, name)
> name, ext = os.path.splitext(name)
> --
> 2.16.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
