@dmnks requested changes on this pull request.
Thanks for the fixup! We're getting there 😄 See my inline comments.
Some notes on the commit message:
* Wrap lines at 72 chars
* Consider using imperative style, such as "Add" rather than "Added" or
"Implement" rather than "Implemented"
> @@ -34,6 +34,17 @@ generated during a build of a package like auto generated
> dependencies.
Invoke an interactive shell for inspecting macros, optionally
after parsing a spec. Mainly useful for troubleshooting.
+*--sources*
+ List all source file URLs defined in the spec file(s).
+ Output separated by newlines.
+
+*--patches*
+ List all patch file URLs defined in the spec file(s).
+ Output separated by newlines.
+
+*--full*
This should actually go into a new section called LIST OPTIONS (below the
existing QUERY OPTIONS) since it's not an *operation*.
Also, the description should be written in imperative style, for example: "Show
full URLs when listing --source or --patch files." (make sure to use the
`*--source*` and `*--patch*` markup here, to conform to the man page style
described in `docs/man/rpm-man-template.scd`).
> @@ -34,6 +34,17 @@ generated during a build of a package like auto generated
> dependencies.
Invoke an interactive shell for inspecting macros, optionally
after parsing a spec. Mainly useful for troubleshooting.
+*--sources*
+ List all source file URLs defined in the spec file(s).
I've just realized that whether URLs are printed depends on the `--full`
switch. So this should rather just say something like "List all source files
defined in the spec file(s).".
> @@ -34,6 +34,17 @@ generated during a build of a package like auto generated
> dependencies.
Invoke an interactive shell for inspecting macros, optionally
after parsing a spec. Mainly useful for troubleshooting.
+*--sources*
+ List all source file URLs defined in the spec file(s).
+ Output separated by newlines.
+
+*--patches*
+ List all patch file URLs defined in the spec file(s).
Same as above.
>
The new operations should also be added here (below this one).
>
It would also be nice to have a simple example here for the newly added
operations.
> };
static int mode = MODE_UNKNOWN;
static int source = RPMQV_SPECRPMS;
+static bool fullPathExpected = 0;
This is cosmetic but maybe just name this `full`. The 'PathExpected' suffix
doesn't add any meaningful information and just makes it longer 😄
> @@ -80,6 +89,32 @@ static int doShell(rpmSpec spec)
}
#endif
+static int printSourceFilenames(rpmSpec spec, rpmSourceFlags what)
+{
+ rpmSpecSrcIter iter = rpmSpecSrcIterInit(spec);
+
+ rpmSpecSrc src;
+ while ((src = rpmSpecSrcIterNext(iter)) != NULL) {
+ if (rpmSpecSrcFlags(src) & what){
+ const char* url = rpmSpecSrcFilename(src, fullPathExpected);
+ if (url) fprintf(stdout, "%s\n", url);
Cosmetic but I'd put the fprinf on a new line below (it's easier to read that
way).
> +[hello-1.0-m<o>dernize&renew.patch
+hello-1.0-inst(a);;.patch
+],
+[])
+
+RPMTEST_CHECK([
+rpmspec --patches ${abs_srcdir}/data/SPECS/hello-sources.spec --full
+],
+[0],
+[hello-1.0-m<o>dernize&renew.patch
+http://example.com/hello-1.0-inst(a);;.patch
+],
+[])
+RPMTEST_CLEANUP
+
+RPMTEST_SETUP([rpmspec --full option without --soruces/patches])
Typo: soruces 😄
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/4262#pullrequestreview-4755586935
You are receiving this because you are subscribed to this thread.
Message ID: <rpm-software-management/rpm/pull/4262/review/[email protected]>_______________________________________________
Rpm-maint mailing list
[email protected]
https://lists.rpm.org/mailman/listinfo/rpm-maint
