Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #310. Add '-f' to 'rm' command in uninstall hooks to avoid error messages if files do not exist. #311

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spuck
Copy link

@spuck spuck commented Nov 16, 2023

Also, quoting directory names in calls to "rm".

…avoid error messages if file does not exist.
@spuck spuck closed this Nov 16, 2023
@spuck spuck reopened this Nov 16, 2023
@atheik
Copy link
Member

atheik commented Nov 17, 2023

@spuck,

Thank you for the PKG_CHECK_VAR fix.

Please leave out your changes to the uninstall-hook targets in this commit.
Giving -f as an option to rm does get rid of the ENOENT messages, but those messages are harmless.
As for quoting the path argument to rm, you are right that the expansion of a user-defined DESTDIR can result in multiple arguments to rm, e.g. when DESTDIR contains spaces, but in-practice most Autotooled projects don't seem to bother with proper quoting. Besides, if you do want to quote them, then quote also the DESTDIR expansions in the install-exec-hook targets.

When in your fix_old_pkg-config branch, you can reset to the latest commit and "undo" it like this:

git reset --hard
git reset HEAD~

You can then remove your changes to the uninstall-hook targets:

git checkout addshare/Makefile.am adduser/Makefile.am control/Makefile.am mountd/Makefile.am

At this point you might fix the indentation in the PKG_CHECK_VAR definition you added to configure.ac.

You can give the commit message from a file by using the -F option of git commit.
In the commit message, the first line is the commit title and should be prefixed with ksmbd-tools: .
After the title, there should be a blank line followed by the rest of the commit message.

The msg file could look like this:

ksmbd-tools: add PKG_CHECK_VAR macro for pre-0.28 pkg-config

The PKG_CHECK_VAR macro was added in pkg-config 0.28, which is too new
for some still-supported distros. Vendor it from pkg-config and use it
if left undefined.

You can then commit these changes with git commit -asF msg and update this PR with git push -f.

If you don't want to bother with updating this PR, then would you consider giving your approval for this commit:

[PATCH] ksmbd-tools: add PKG_CHECK_VAR macro for pre-0.28 pkg-config (click the arrow to expand)
From 782d583e1dad8e17bf7993df0f7aba369ae00d1d Mon Sep 17 00:00:00 2001
From: Travis Hayes <[email protected]>
Date: Fri, 17 Nov 2023 02:29:23 +0200
Subject: [PATCH] ksmbd-tools: add PKG_CHECK_VAR macro for pre-0.28 pkg-config

The PKG_CHECK_VAR macro was added in pkg-config 0.28, which is too new
for some still-supported distros. Vendor it from pkg-config and use it
if left undefined.

Signed-off-by: Travis Hayes <[email protected]>
---
 configure.ac | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/configure.ac b/configure.ac
index de5158b..76935e3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,23 @@ AC_PROG_SED
 AC_PROG_MKDIR_P
 AC_PROG_LN_S
 
+# Check if PKG_CHECK_VAR exists for compatibility with older (pre-version 0.28) pkg-config
+m4_ifndef([PKG_CHECK_VAR], [
+	# PKG_CHECK_VAR(VARIABLE, MODULE, CONFIG-VARIABLE,
+	# [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
+	# -------------------------------------------
+	# Retrieves the value of the pkg-config variable for the given module.
+	AC_DEFUN([PKG_CHECK_VAR],
+	[AC_REQUIRE([PKG_PROG_PKG_CONFIG])dnl
+	AC_ARG_VAR([$1], [value of $3 for $2, overriding pkg-config])dnl
+
+	_PKG_CONFIG([$1], [variable="][$3]["], [$2])
+	AS_VAR_COPY([$1], [pkg_cv_][$1])
+
+	AS_VAR_IF([$1], [""], [$5], [$4])dnl
+	])# PKG_CHECK_VAR
+])
+
 AC_SUBST([in_script], [[\
 '$(SED) -e "s,[@]sbindir[@],$(sbindir),g" \
         -e "s,[@]sysconfdir[@],$(sysconfdir),g" \
-- 
2.42.1

I have not extensively tested this but I trust you have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants