Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
Merged #2992 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#event-12247199537 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
...and now with docs... -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#issuecomment-2019946369 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai pushed 4 commits. b5cb2b48af7293c3b1608b1cef289fed1b14ad2c Use rpmGlobPath(... RPMGLOB_NOCHECK) for rpmrc reading d48bf2e1e3e6e3512401e83d04d38eb6807e4480 Sanitize rpmGlob() behavior wrt non-glob patterns de0ae1121392e50aa59032e5d2e6d97d65e8bd4f Drop support for compile-time MACROFILES override 952310439b1431b14ef041fde2b6d692530d0132 Support per-user macro configuration in XDG_CONFIG_HOME -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992/files/6eaa933386a72d4a5673e7e86969bca679cbb6a8..952310439b1431b14ef041fde2b6d692530d0132 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
Ugh, right... -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#issuecomment-2019920673 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
Eep, another thing this is missing: docs. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#issuecomment-2019919241 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
Oh, and please add a label for the category of this feature (for the future auto-changelog). -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#issuecomment-2019913634 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@dmnks approved this pull request. Looks good now! And thanks for the updated commit message (wrt the "rpm --showrc" case) :smile: -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#pullrequestreview-1959771873 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request.
> +}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
+
+if (rpmGlob(dotpath, NULL, NULL)) {
+ const char *oldcfg = "~/.rpmmacros";
+ if (rpmGlob(oldcfg, NULL, NULL) == 0) {
It's also a good reminder of: document not only what, but also *why* - the
latter was missing in the original commit message.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538852454
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@dmnks commented on this pull request.
> +}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
+
+if (rpmGlob(dotpath, NULL, NULL)) {
+ const char *oldcfg = "~/.rpmmacros";
+ if (rpmGlob(oldcfg, NULL, NULL) == 0) {
Oh! This is a good point and something that completely eluded me. Thanks for
the explanation!
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538842481
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request.
> +#ifdef MACROFILES
+static char *initMacroPath(const char *confdir)
+{
+return xstrdup(MACROFILES);
+}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
Actually the ~/.rpmrc thingie did cross my mind yesterday but I just dismissed
the warning light.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538732979
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
Updated to use ~/.config/rpm directory and take ~/.rpmrc into account as well. Thanks guys for the feedback! -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#issuecomment-2019689559 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request. > @@ -2,6 +2,41 @@ # AT_BANNER([RPM macros]) +AT_SETUP([macro path]) +AT_KEYWORDS([macros]) +RPMDB_INIT + +# .rpmmacros exists Right, this ... crossed my mind but dismissed it, along with several other warning bells related to this PR, too busy getting it done. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538730963 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai pushed 2 commits. 6ee9eedc9af522bbd0025b0874222477ccf40139 Drop support for compile-time MACROFILES override 6eaa933386a72d4a5673e7e86969bca679cbb6a8 Support per-user macro configuration in XDG_CONFIG_HOME -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992/files/1172a0f60e336891a86df77de89e3e82d536b39f..6eaa933386a72d4a5673e7e86969bca679cbb6a8 You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request.
> +#ifdef MACROFILES
+static char *initMacroPath(const char *confdir)
+{
+return xstrdup(MACROFILES);
+}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
Come to think of it, not just future: there's already ~/.rpmrc that we should
support in the new location too. Sigh :smile:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538670584
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request.
> +}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
+
+if (rpmGlob(dotpath, NULL, NULL)) {
+ const char *oldcfg = "~/.rpmmacros";
+ if (rpmGlob(oldcfg, NULL, NULL) == 0) {
The second glob is a bit subtle, but it's quite intentional as per the commit
message (emphasis added): "fall back to traditional ~/.rpmmacros *iff* it
exists and there's no config in the XDG location."
Without the second glob, we'd advertise the old location in macro path if no
per-user configuration is found, whereas we want to advertise the new location
going forward. I can clarify this in the commit message.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538667704
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai commented on this pull request.
> +#ifdef MACROFILES
+static char *initMacroPath(const char *confdir)
+{
+return xstrdup(MACROFILES);
+}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
It also leaves room for future additions, so why not - will do.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1538660848
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@dmnks commented on this pull request.
> +#ifdef MACROFILES
+static char *initMacroPath(const char *confdir)
+{
+return xstrdup(MACROFILES);
+}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
+1 for the above path, too. It's also more conventional, as it seems.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#discussion_r1537906333
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@dmnks requested changes on this pull request.
> @@ -2,6 +2,41 @@
#
AT_BANNER([RPM macros])
+AT_SETUP([macro path])
+AT_KEYWORDS([macros])
+RPMDB_INIT
+
+# .rpmmacros exists
I'd suggest that we actually check that `~/.rpmmacros` really exists (or just
`touch` it like in the other tests below). It's currently basically an
[implementation
detail](https://github.com/rpm-software-management/rpm/blob/e16bf1fb3198ef0b4c5a1644b627b91db13da99b/tests/Dockerfile.fedora#L55)
of the test image that, if ever changes, would cause a weird test failure
:smile:
> +}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
+
+if (rpmGlob(dotpath, NULL, NULL)) {
+ const char *oldcfg = "~/.rpmmacros";
+ if (rpmGlob(oldcfg, NULL, NULL) == 0) {
Cosmetic: This second `rpmGlob()` check doesn't really achieve anything; if
`~/.rpmmacros` is missing, we'll just keep (the also missing)
`~/.config/rpmmacros` in `dotpath`. When later loading the macro files in
`rpmInitMacros()`, we'll just skip over missing ones anyway.
Note that if you remove this check, though, you'll need to update the test
covering the `XDG_CONFIG_HOME` use case (to actually create the
`~/.zzz/rpmconfig` file).
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#pullrequestreview-1957907594
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@Conan-Kudo requested changes on this pull request.
> +#ifdef MACROFILES
+static char *initMacroPath(const char *confdir)
+{
+return xstrdup(MACROFILES);
+}
+#else
+/*
+ * Prefer XDG_CONFIG_HOME/rpmmacros but fall back to ~/.rpmmacros
+ * if it exists and the XDG path doesn't.
+ */
+static char *initMacroPath(const char *confdir)
+{
+const char *xdgconf = getenv("XDG_CONFIG_HOME");
+if (!(xdgconf && *xdgconf))
+ xdgconf = "~/.config";
+char *dotpath = rpmGetPath(xdgconf, "/rpmmacros", NULL);
What about `$XDG_CONFIG_HOME/rpm/macros` instead? That mimics what we do in
`/etc/rpm` too.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2992#pullrequestreview-1958109674
You are receiving this because you are subscribed to this thread.
Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Support per-user macro configuration in XDG_CONFIG_HOME (PR #2992)
@pmatilai pushed 3 commits. ee17de74cf23df317e9c53f259c9bdd483a2ff4d Use rpmGlobPath(... RPMGLOB_NOCHECK) for rpmrc reading 3b796dc90e819fe7cfaa621cd7e8d4f62c6c517d Sanitize rpmGlob() behavior wrt non-glob patterns 1172a0f60e336891a86df77de89e3e82d536b39f Support per-user macro configuration in XDG_CONFIG_HOME -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2992/files/03490923d27afb018990b9c834f6594de8e77607..1172a0f60e336891a86df77de89e3e82d536b39f You are receiving this because you are subscribed to this thread. Message ID: ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
