vim: install /bin/vi as a relative symlink
PATCH 1/1 vim: install /bin/vi as a relative symlink
Prevent creating a dangling symlink when vim is not present on the host machine. With BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".
--- package/vim/vim.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
package/vim/vim.mk +7 -1
@@ -63,9 +63,15 @@ define VIM_REMOVE_DOCS endef # Avoid oopses with vipw/vigr, lack of $EDITOR and 'vi' command expectation +ifeq ($(BR2_ROOTFS_MERGED_USR),y) define VIM_INSTALL_VI_SYMLINK - ln -sf /usr/bin/vim $(TARGET_DIR)/bin/vi + ln -sf vim $(TARGET_DIR)/usr/bin/vi endef +else +define VIM_INSTALL_VI_SYMLINK + ln -sf ../usr/bin/vim $(TARGET_DIR)/bin/vi +endef +endif VIM_POST_INSTALL_TARGET_HOOKS += VIM_INSTALL_VI_SYMLINK ifeq ($(BR2_PACKAGE_VIM_RUNTIME),y)
Signature · 1 line
2.14.4
PATCH 1/1 vim: install /bin/vi as a relative symlink
Hello,
Quoted 5 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".Signed-off-by Carlos Santos <[email protected]>
Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
Thanks,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 14 lines
From: "Thomas Petazzoni" <[email protected]>
To: "DATACOM" <[email protected]>
Cc: "buildroot" <[email protected]>, "Yann Morin" <[email protected]>, "ratbert90" <[email protected]>, "Baruch Siach" <[email protected]>
Sent: Wednesday, July 18, 2018 10:03:50 AM
Subject: Re: [Buildroot] [PATCH] vim: install/bin/vias a relative symlinkHello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".
Quoted 7 lines
Quoted 1 line
Signed-off-by Carlos Santos <[email protected]>Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
Baruch's patch is for busybox and is under review. Both can be applied independently.
Signature · 3 lines
Carlos Santos (Casantos) - DATACOM, P&D ?Marched towards the enemy, spear upright, armed with the certainty that only the ignorant can have.? ? Epitaph of a volunteer
PATCH 1/1 vim: install /bin/vi as a relative symlink
In reply to Thomas PetazzoniHi Thomas,
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".
Quoted 7 lines
Quoted 1 line
Signed-off-by Carlos Santos <[email protected]>Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
My vim patch is at
http://patchwork.ozlabs.org/patch/943314/
I marked it as Rejected following the comment of Arnout. Carlos' patch works around the merged /usr issue by changing the symlink target for merged /usr. In my opinion this solution is error prone. It would be much easier to allow dangling symlinks in the target directory, and tweak the busybox install.sh to cope with that. That's what my pending busybox patch suggests.
http://patchwork.ozlabs.org/patch/944884/
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 25 lines
From: "Baruch Siach" <[email protected]>
To: "Thomas Petazzoni" <[email protected]>
Cc: "DATACOM" <[email protected]>, "buildroot" <[email protected]>, "Yann Morin" <[email protected]>, "ratbert90" <[email protected]>, "Arnout Vandecappelle" <[email protected]>
Sent: Wednesday, July 18, 2018 11:57:06 PM
Subject: Re: [Buildroot] [PATCH] vim: install/bin/vias a relative symlinkHi Thomas,
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".Quoted 7 lines
Quoted 1 line
Signed-off-by Carlos Santos <[email protected]>Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
Quoted 7 lines
My vim patch is at
http://patchwork.ozlabs.org/patch/943314/
I marked it as Rejected following the comment of Arnout. Carlos' patch works around the merged /usr issue by changing the symlink target for merged /usr. In my opinion this solution is error prone.
Why?
Quoted 6 lines
It would be
much easier to allow dangling symlinks in the target directory, and tweak the busyboxinstall.shto cope with that. That's what my pending busybox patch suggests.
Both patches could be applied. They are independent from each other.
Signature · 3 lines
Carlos Santos (Casantos) - DATACOM, P&D ?Marched towards the enemy, spear upright, armed with the certainty that only the ignorant can have.? ? Epitaph of a volunteer
PATCH 1/1 vim: install /bin/vi as a relative symlink
Hi Carlos,
Quoted 27 lines · Carlos Santos
Quoted 27 lines
From: "Baruch Siach" <[email protected]>
To: "Thomas Petazzoni" <[email protected]>
Cc: "DATACOM" <[email protected]>, "buildroot" <[email protected]>, "Yann Morin" <[email protected]>, "ratbert90" <[email protected]>, "Arnout Vandecappelle" <[email protected]>
Sent: Wednesday, July 18, 2018 11:57:06 PM
Subject: Re: [Buildroot] [PATCH] vim: install/bin/vias a relative symlinkHi Thomas,
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".Quoted 7 lines
Quoted 1 line
Signed-off-by Carlos Santos <[email protected]>Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
My vim patch is at
Quoted 7 lines
Quoted 5 lines
http://patchwork.ozlabs.org/patch/943314/
I marked it as Rejected following the comment of Arnout. Carlos' patch works around the merged /usr issue by changing the symlink target for merged /usr. In my opinion this solution is error prone.
Why?
Because it is confusing, for me at least. Changes in the filesystem layout adding or removing directory symlinks might break them.
Quoted 8 lines
Quoted 6 lines
It would be
much easier to allow dangling symlinks in the target directory, and tweak the busyboxinstall.shto cope with that. That's what my pending busybox patch suggests.Both patches could be applied. They are independent from each other.
But if we allow dangling non-relative symlinks I don't think we want to add this complexity to the vim package, or any other package that installs similar relative symlinks.
baruch
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 40 lines
From: "Baruch Siach" <[email protected]>
To: "DATACOM" <[email protected]>
Cc: "Thomas Petazzoni" <[email protected]>, "buildroot" <[email protected]>, "Yann Morin" <[email protected]>, "ratbert90" <[email protected]>, "Arnout Vandecappelle" <[email protected]>
Sent: Friday, July 20, 2018 2:58:27 AM
Subject: Re: [Buildroot] [PATCH] vim: install/bin/vias a relative symlinkHi Carlos,
Quoted 30 lines · Carlos Santos
Quoted 30 lines
From: "Baruch Siach" <[email protected]>
To: "Thomas Petazzoni" <[email protected]>
Cc: "DATACOM" <[email protected]>, "buildroot" <[email protected]>, "Yann Morin" <[email protected]>,
"ratbert90" <[email protected]>, "Arnout Vandecappelle" <[email protected]>
Sent: Wednesday, July 18, 2018 11:57:06 PM
Subject: Re: [Buildroot] [PATCH] vim: install/bin/vias a relative symlinkHi Thomas,
Quoted 7 lines · Thomas Petazzoni
Hello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".Quoted 7 lines
Quoted 1 line
Signed-off-by Carlos Santos <[email protected]>Baruch had already sent a patch with the same title/intention, but it is no longer in the pending state in patchwork.
Could you clarify what happened, and which of the two patches is relevant ?
My vim patch is at
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
I marked it as Rejected following the comment of Arnout. Carlos' patch works around the merged /usr issue by changing the symlink target for merged /usr. In my opinion this solution is error prone.
Why?
Because it is confusing, for me at least. Changes in the filesystem layout adding or removing directory symlinks might break them.
Anyone that removes the directory symlinks is looking for trouble.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
It would be
much easier to allow dangling symlinks in the target directory, and tweak the busyboxinstall.shto cope with that. That's what my pending busybox patch suggests.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Both patches could be applied. They are independent from each other.
But if we allow dangling non-relative symlinks I don't think we want to add this complexity to the vim package, or any other package that installs similar relative symlinks.
The complexity is a consequence of the BR2_ROOTFS_MERGED_USR stuff. We already need to deal with such situations in six packages. I'd rather declare merged /usr as mandatory and send all those ifeq's down the tubes.
Signature · 3 lines
Carlos Santos (Casantos) - DATACOM, P&D ?Marched towards the enemy, spear upright, armed with the certainty that only the ignorant can have.? ? Epitaph of a volunteer
PATCH 1/1 vim: install /bin/vi as a relative symlink
Hello,
Quoted 8 lines · Carlos Santos
Quoted 3 lines
But if we allow dangling non-relative symlinks I don't think we want to add this complexity to the vim package, or any other package that installs similar relative symlinks.
The complexity is a consequence of the
BR2_ROOTFS_MERGED_USRstuff. We already need to deal with such situations in six packages. I'd rather declare merged /usr as mandatory and send all those ifeq's down the tubes.
Perhaps we could introduce one or two helper functions that hide what BR2_ROOTFS_MERGED_USR is doing. I thought a few minutes about this and couldn't find immediately a good semantic/naming, but perhaps this is a direction that could be investigated ?
Best regards,
Thomas Petazzoni
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 13 lines · Thomas Petazzoni
Hello,
Quoted 8 lines · Carlos Santos
Quoted 3 lines
But if we allow dangling non-relative symlinks I don't think we want to add this complexity to the vim package, or any other package that installs similar relative symlinks.
The complexity is a consequence of the
BR2_ROOTFS_MERGED_USRstuff. We already need to deal with such situations in six packages. I'd rather declare merged /usr as mandatory and send all those ifeq's down the tubes.
Quoted 4 lines
Perhaps we could introduce one or two helper functions that hide what
BR2_ROOTFS_MERGED_USRis doing. I thought a few minutes about this and couldn't find immediately a good semantic/naming, but perhaps this is a direction that could be investigated ?
The thing is, I don't believe that relying on BR2_ROOTFS_MERGED_USR is the
right approach. I've had projects with a usr -> . symlink in a custom skeleton, for instance. We could of course declare such a skeleton invalid, but I don't think that it is checked currently. Also I can imagine that there could be other custom skeleton layouts that could create problems.
We already use ln --relative in a few places, isn't that the easiest solution?
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 vim: install /bin/vi as a relative symlink
Hello,
Quoted 7 lines · Arnout Vandecappelle
The thing is, I don't believe that relying on
BR2_ROOTFS_MERGED_USRis the
right approach. I've had projects with a usr -> . symlink in a custom skeleton, for instance. We could of course declare such a skeleton invalid, but I don't think that it is checked currently. Also I can imagine that there could be other custom skeleton layouts that could create problems.We already use ln --relative in a few places, isn't that the easiest solution?
ln --relative is not supported on old distros, we even have a patch on systemd to remove its use of ln --relative.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 10 lines · Thomas Petazzoni
Hello,
Quoted 6 lines · Arnout Vandecappelle
The thing is, I don't believe that relying on
BR2_ROOTFS_MERGED_USRis the
right approach. I've had projects with a usr -> . symlink in a custom skeleton, for instance. We could of course declare such a skeleton invalid, but I don't think that it is checked currently. Also I can imagine that there could be other custom skeleton layouts that could create problems.
Quoted 4 lines
Quoted 1 line
We already use ln --relative in a few places, isn't that the easiest solution?
ln --relative is not supported on old distros, we even have a patch on systemd to remove its use of ln --relative.
Crap, yes, I had done a quick grep, found some hits, and didn't notice it were
patches *removing* the --relative.
I would still very much prefer to create a relative symlink automagically. Some
ideas:
1. Install a proper host-coreutils if the system-provided one is too old (similar to cmake). Meh.
2. Always make absolute symlinks (pointing to target dir) and at some point call
https://github.com/brandt/symlinks to convert absolute to relative.
3. Make an 'ln' wrapper that converts absolute to relative, using e.g. the shell scriptlet from [1].
Regards,
Arnout
[1]
https://unix.stackexchange.com/questions/85060/getting-relative-links-between-two-paths/85068#85068
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/1 vim: install /bin/vi as a relative symlink
In reply to Baruch SiachHello,
Quoted 4 lines · Baruch Siach
My vim patch is at
Quoted 8 lines
I marked it as Rejected following the comment of Arnout. Carlos' patch works around the merged /usr issue by changing the symlink target for merged /usr. In my opinion this solution is error prone. It would be much easier to allow dangling symlinks in the target directory, and tweak the busybox
install.shto cope with that. That's what my pending busybox patch suggests.
Allowing dangling symlinks is indeed desirable, but I think it is also nice if we have as few dangling symlinks are possible. It looks cleaner to me. So like Carlos said, I believe both your patch fixing the Busybox installation and Carlos patch adjusting vim are useful.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/1 vim: install /bin/vi as a relative symlink
In reply to Carlos SantosHello,
Quoted 8 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".Signed-off-by Carlos Santos <[email protected]>--- package/vim/vim.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
I know there is some ongoing discussion on how to handle this better in a more generic way, but until those discussions are resolved, I've applied this patch to master.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/1 vim: install /bin/vi as a relative symlink
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hello,
Quoted 4 lines · Carlos Santos
Prevent creating a dangling symlink when vim is not present on the host machine. With
BR2_ROOTFS_MERGED_USR, just link to "vim", since they are on the same directory, otherwise link to "../usr/bin/vim".
Quoted 8 lines
Quoted 4 lines
Signed-off-by Carlos Santos <[email protected]>---
package/vim/vim.mk| 8 +++++++-1 file changed, 7 insertions(+), 1 deletion(-)I know there is some ongoing discussion on how to handle this better in a more generic way, but until those discussions are resolved, I've applied this patch to master.
Committed to 2018.02.x and 2018.05.x, thanks.
Signature · 1 line
Bye, Peter Korsgaard