Page MenuHomeFreeBSD

Delete the leftover ${WRKSRC}/.metadir directory when 'make install' fails for any reason
ClosedPublic

Authored by mat on Jan 8 2018, 6:04 PM.
Tags
None
Referenced Files
F102654846: D13798.id38752.diff
Fri, Nov 15, 10:08 AM
Unknown Object (File)
Mon, Oct 28, 3:04 PM
Unknown Object (File)
Oct 16 2024, 3:17 AM
Unknown Object (File)
Sep 20 2024, 8:39 AM
Unknown Object (File)
Sep 15 2024, 4:40 AM
Unknown Object (File)
Sep 9 2024, 1:50 AM
Unknown Object (File)
Sep 8 2024, 8:14 PM
Unknown Object (File)
Sep 8 2024, 12:45 AM
Subscribers

Diff Detail

Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 14578
Build 14712: arc lint + arc unit

Event Timeline

This problem happens when during development 'make install' fails due to, for example, a duplicate file conflict.

yuri retitled this revision from Deletes the leftover ${WRKSRC}/.metadir directory when 'make install' fails for any reason to Delete the leftover ${WRKSRC}/.metadir directory when 'make install' fails for any reason.Jan 8 2018, 10:33 PM

I don't think this is the right fix.

In D13798#289393, @mat wrote:

I don't think this is the right fix.

  1. It accomplishes the cleanup, which is obviously missing in this place
  2. Tests confirm that it solves the original problem.

So, what is exactly not right about it?

These files do not need root permissions. I suggest we fix THAT.

These files do not need root permissions. I suggest we fix THAT.

Once the make install command runs as root, it creates them with root credentials. It is possible to create them with credentials matching the work directory. But besides looking a bit stilted, this also has a downside of leaving these temporary files in case of failure.

I've run into this bug many times before. Thank you for bringing this up Yuri.

In D13798#294776, @yuri wrote:

These files do not need root permissions. I suggest we fix THAT.

Once the make install command runs as root, it creates them with root credentials.

This begs the question if create-manifest really needs to be in _INSTALL_SUSEQ (as part of fake-pkg) or if it can simply be in _INSTALL_SEQ?

Something like

diff --git a/Mk/bsd.port.mk b/Mk/bsd.port.mk
index 3db728b11168..cd6f75c10fd8 100644
--- a/Mk/bsd.port.mk
+++ b/Mk/bsd.port.mk
@@ -4671,7 +4671,7 @@ flavors-package-names: .PHONY
 STAGE_ARGS=		-i ${STAGEDIR}
 
 .if !defined(NO_PKG_REGISTER)
-fake-pkg: create-manifest
+fake-pkg:
 .if defined(INSTALLS_DEPENDS)
 	@${ECHO_MSG} "===>   Registering installation for ${PKGNAME} as automatic"
 .else
@@ -5410,8 +5410,9 @@ _TEST_SEQ=		100:test-message 150:test-depends 300:pre-test 500:do-test \
 				${_OPTIONS_test} ${_USES_test}
 _INSTALL_DEP=	stage
 _INSTALL_SEQ=	100:install-message \
-				200:check-already-installed
-_INSTALL_SUSEQ=	300:fake-pkg 500:security-check
+				200:check-already-installed \
+				300:create-manifest
+_INSTALL_SUSEQ=	301:fake-pkg 500:security-check
 
 _PACKAGE_DEP=	stage
 _PACKAGE_SEQ=	100:package-message 300:pre-package 450:pre-package-script \

I've run into this bug many times before. Thank you for bringing this up Yuri.

In D13798#294776, @yuri wrote:

These files do not need root permissions. I suggest we fix THAT.

Once the make install command runs as root, it creates them with root credentials.

This begs the question if create-manifest really needs to be in _INSTALL_SUSEQ (as part of fake-pkg) or if it can simply be in _INSTALL_SEQ?

I don't have time to review at the moment but I think you're right that it doesn't need to be in SUSEQ.

In D13798#289472, @yuri wrote:
In D13798#289393, @mat wrote:

I don't think this is the right fix.

  1. It accomplishes the cleanup, which is obviously missing in this place
  2. Tests confirm that it solves the original problem.

So, what is exactly not right about it?

Because you're fixing the symptoms, not the cause.

mat added a reviewer: yuri.
mat retitled this revision from Delete the leftover ${WRKSRC}/.metadir directory when 'make install' fails for any reason to Don't run create-manifest as root.
mat retitled this revision from Don't run create-manifest as root to Delete the leftover ${WRKSRC}/.metadir directory when 'make install' fails for any reason.

Take on the patch from @tobik.

Tested on a couple of ports, I cannot see any harm it can do.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 1 2018, 8:31 AM
This revision was automatically updated to reflect the committed changes.