Re: [Rpm-maint] [rpm-software-management/rpm] No build directives in generated spec parts (PR #2917)
Merged #2917 into master. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917#event-12039458910 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] No build directives in generated spec parts (PR #2917)
@pmatilai approved this pull request. I actually meant you could just as well inline the whole lookup in what you're calling checkPart() since there are no other users for this in sight, but works for me. -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1919782341 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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> + return p->token;
+ }
+}
+return NULL;
+}
+
+static const int partBeforeBuildOnly(int part)
+{
+const struct PartRec *p;
+
+for (p = partList; p->token != NULL; p++) {
+ if (p->part == part) {
+ return p->beforebuildonly;
+ }
+}
+return 0;
Not sure if the new code is that more readable. But whatever...
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514452432
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] No build directives in generated spec parts (PR #2917)
@ffesti pushed 1 commit. 137a4353cf32f7a4d10d05d216dbd9392f84acc4 Don't allow build directives in generated specs -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917/files/8dcf8824b2e7c00097e28e7ac1d4e939e431a0ab..137a4353cf32f7a4d10d05d216dbd9392f84acc4 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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> + return p->token;
+ }
+}
+return NULL;
+}
+
+static const int partBeforeBuildOnly(int part)
+{
+const struct PartRec *p;
+
+for (p = partList; p->token != NULL; p++) {
+ if (p->part == part) {
+ return p->beforebuildonly;
+ }
+}
+return 0;
Oh, I see you mentioned this below. isPart() is a different matter, but ..
Once you have
```
static const struct PartRec * getPart(int part);
```
...the check becomes something like:
```
if (stage == PARSE_GENERATED) {
const struct PartRec *p = getPart(parsePart);
if (p && p->prebuildonly) {
rpmlog(RPMLOG_ERR,
_("Section %s is not allowed after build is done!\n"),
p->token);
goto errxit;
}
}
```
...and at that point you realize this whole if belongs into a separate helper
function that checks whether the tag was legitimate for use here.
parseSpecSection() is big enough as it is.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514412924
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> + return p->token;
+ }
+}
+return NULL;
+}
+
+static const int partBeforeBuildOnly(int part)
+{
+const struct PartRec *p;
+
+for (p = partList; p->token != NULL; p++) {
+ if (p->part == part) {
+ return p->beforebuildonly;
+ }
+}
+return 0;
As I wrote above this is not quite as simple as it looks and I ma not that
interested in refactoring the whole parting code. Yes, it looks wasteful and
over complicated but I'd rather not change a whole bunch of functions just to
pass a new pointer around.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514406101
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] No build directives in generated spec parts (PR #2917)
Oh and for whatever reason, there's some po/ submodule update in here: > diff --git a/po b/po index d5cc5d368e..eee506492f 16 --- a/po +++ b/po @@ -1 +1 @@ -Subproject commit d5cc5d368e2cbb639156b3f6ceb824a8815fdeba +Subproject commit eee506492f5fe2f4fe8940c5e5b088b42167b790 (also visible in the changed files tab) -- Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917#issuecomment-1980770691 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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> + return p->token;
+ }
+}
+return NULL;
+}
+
+static const int partBeforeBuildOnly(int part)
+{
+const struct PartRec *p;
+
+for (p = partList; p->token != NULL; p++) {
+ if (p->part == part) {
+ return p->beforebuildonly;
+ }
+}
+return 0;
There's still this one :innocent:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514385888
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> @@ -1060,71 +1069,72 @@ typedef const struct PreambleRec_s {
int type;
int deprecated;
int ismacro;
+int beforebuildonly;
`prebuildonly` it is. Not that I am looking for any hills...
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514373998
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> {0, 0, 0, 0}
};
/**
*/
static int findPreambleTag(rpmSpec spec,rpmTagVal * tag,
- const char ** macro, char * lang)
+ const char ** macro, char * lang, int * beforebuildonly)
Done. I looked at the same issue with `partName`, `partPrebuildonly` and
`isPart` but there is is much more difficult to return the struct, as the
PART_* value is handed througfh all kind of layers. So I keep things the way
they are there.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1514373256
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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> @@ -1060,71 +1069,72 @@ typedef const struct PreambleRec_s {
int type;
int deprecated;
int ismacro;
+int beforebuildonly;
This is certainly better than the not-version (negations in variable names are
the wrong thing, every time), just looks somehow out of place in that company.
"prebuild" maybe? Not that this name is a hill to die for :laughing:
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1916104539
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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> {0, 0, 0, 0}
};
/**
*/
static int findPreambleTag(rpmSpec spec,rpmTagVal * tag,
- const char ** macro, char * lang)
+ const char ** macro, char * lang, int * beforebuildonly)
findPreambleTag() is of course pre-existing code that just gets extended here
so this is just a side-remark, but this function sure looks like it'd be much
saner to just return a pointer to the struct instead of all these separate
return pointers.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1908639748
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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> + return p->token;
+ }
+}
+return NULL;
+}
+
+static const int partBeforeBuildOnly(int part)
+{
+const struct PartRec *p;
+
+for (p = partList; p->token != NULL; p++) {
+ if (p->part == part) {
+ return p->beforebuildonly;
+ }
+}
+return 0;
I'd combine these two into findPart() that returns a pointer to the struct,
whose values you can then just use directly in the caller.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1908632646
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> @@ -1109,6 +1123,25 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int
> secondary)
goto errxit;
}
+ if (stage == PARSE_GENERATED) {
+ switch (parsePart) {
+ case PART_PREP:
+ case PART_BUILD:
+ case PART_INSTALL:
+ case PART_CHECK:
+ case PART_CLEAN: /* */
+ case PART_BUILDARCHITECTURES:
+ case PART_PATCHLIST:
+ case PART_SOURCELIST:
+ case PART_BUILDREQUIRES:
+ case PART_CONF:
Yes, it was. But I also was confused when seeing it and at first thought the
rebase went wrong.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1507168561
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] No build directives in generated spec parts (PR #2917)
@ffesti pushed 1 commit. 1e89c45a1336b3927caa5222a536d2864b1c0c16 Don't allow build directives in generated specs -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917/files/b1eb41516fd8a6674f334502f98a99d3aa7f9a58..1e89c45a1336b3927caa5222a536d2864b1c0c16 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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> @@ -1060,71 +1069,72 @@ typedef const struct PreambleRec_s {
int type;
int deprecated;
int ismacro;
+int notgenerated;
"notgenerated" is not the greatest name ever :smile:
Maybe "isgenerated" and just flip the meaning around? Other possibilities could
be something around dynamic/static.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1905882276
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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> @@ -1109,6 +1123,25 @@ static rpmRC parseSpecSection(rpmSpec *specptr, int
> secondary)
goto errxit;
}
+ if (stage == PARSE_GENERATED) {
+ switch (parsePart) {
+ case PART_PREP:
+ case PART_BUILD:
+ case PART_INSTALL:
+ case PART_CHECK:
+ case PART_CLEAN: /* */
+ case PART_BUILDARCHITECTURES:
+ case PART_PATCHLIST:
+ case PART_SOURCELIST:
+ case PART_BUILDREQUIRES:
+ case PART_CONF:
Not sure if this was in the earlier version or I just missed it, but this info
should go to struct PartRec for the same reason as the tags: to force people to
consider the matter when adding new.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1905879453
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> + if (stage == GENERATED) {
+ switch (tag) {
+ case RPMTAG_SOURCE:
+ case RPMTAG_PATCH:
+ case RPMTAG_NOSOURCE:
+ case RPMTAG_NOPATCH:
+ case RPMTAG_EXCLUDEARCH:
+ case RPMTAG_EXCLUSIVEARCH:
+ case RPMTAG_EXCLUDEOS:
+ case RPMTAG_EXCLUSIVEOS:
+ case RPMTAG_BUILDROOT:
+ case RPMTAG_BUILDCONFLICTS:
+ case RPMTAG_BUILDOPTION:
+ case RPMTAG_BUILDPREREQ:
+ case RPMTAG_BUILDREQUIRES:
+ case RPMTAG_BUILDSYSTEM:
Done.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1505546709
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] No build directives in generated spec parts (PR #2917)
@ffesti commented on this pull request.
> @@ -21,6 +21,11 @@
#define ALLOWED_CHARS_EVR ALLOWED_CHARS_VERREL "-:"
#define LEN_AND_STR(_tag) (sizeof(_tag)-1), (_tag)
+enum parseStages {
+SPECFILE,
+BUILDSYS,
+GENERATED,
Done
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#discussion_r1505546294
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] No build directives in generated spec parts (PR #2917)
@ffesti pushed 2 commits. 69b688ed04e78f40c9565f570e5fb72b8f64a057 Add parseStages enum b1eb41516fd8a6674f334502f98a99d3aa7f9a58 No build directives in generated spec parts -- View it on GitHub: https://github.com/rpm-software-management/rpm/pull/2917/files/55c12c3e283d39e412642e317cd89a2a5d577e0a..b1eb41516fd8a6674f334502f98a99d3aa7f9a58 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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> @@ -21,6 +21,11 @@
#define ALLOWED_CHARS_EVR ALLOWED_CHARS_VERREL "-:"
#define LEN_AND_STR(_tag) (sizeof(_tag)-1), (_tag)
+enum parseStages {
+SPECFILE,
+BUILDSYS,
+GENERATED,
Use some unique prefix on the enum names, makes it easier to grep for affected
places, something like STAGE_FOO or PARSE_FOO.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1890141434
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] No build directives in generated spec parts (PR #2917)
@pmatilai commented on this pull request.
> + if (stage == GENERATED) {
+ switch (tag) {
+ case RPMTAG_SOURCE:
+ case RPMTAG_PATCH:
+ case RPMTAG_NOSOURCE:
+ case RPMTAG_NOPATCH:
+ case RPMTAG_EXCLUDEARCH:
+ case RPMTAG_EXCLUSIVEARCH:
+ case RPMTAG_EXCLUDEOS:
+ case RPMTAG_EXCLUSIVEOS:
+ case RPMTAG_BUILDROOT:
+ case RPMTAG_BUILDCONFLICTS:
+ case RPMTAG_BUILDOPTION:
+ case RPMTAG_BUILDPREREQ:
+ case RPMTAG_BUILDREQUIRES:
+ case RPMTAG_BUILDSYSTEM:
Add this info to the struct in preambleList instead, that forces one to
consider this issue whenever new tags are added.
You can check for this from findPreambleTag() just as well.
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2917#pullrequestreview-1890134783
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
