Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Fantastic, thank you very much! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-496427088___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Awesome. Thank you, guys! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-496388368___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Merged #593 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#event-2370694142___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai approved this pull request. Looking fine now. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-242466458___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Okay, I think that was all. BTW @ffesti, please add at least a brief comment when pushing updates, I only noticed this had been updated by accident this morning when it could've been merged yesterday already. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-496381039___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Some minor tweaks to do still: - Squash the tests to the commit that adds the feature, that's where they belong - There's an way too long parseRCPOT() line in doBuildRequires() for loop still, not sure if it's been mentioned before, if not sorry for missing on previous rounds - Clean up the test spec a bit, please. We shouldn't really add any *new* references of these... ``` +Vendor: Red Hat Software +Packager: Red Hat Software ... +%changelog +* Tue Oct 20 1998 Jeff Johnson +- create. ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-496186410___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Ok, --quiet issue is fixed. Thank to Igor for helping with this! Test are also added - and should now also include the new spec and tar ball... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-496176939___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ignatenkobrain commented on this pull request.
> @@ -228,12 +229,13 @@ static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec
> spec, int test)
rpmps ps = rpmSpecCheckDeps(ts, spec);
if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-if (ps != NULL)
+ if (rpmIsVerbose()) {
+ rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
Fixed!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r287447752___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -228,12 +229,13 @@ static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec
> spec, int test)
rpmps ps = rpmSpecCheckDeps(ts, spec);
if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-if (ps != NULL)
+ if (rpmIsVerbose()) {
+ rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
This newly added condition is wrong, errors and warnings must be emitted in
--quiet mode too.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-241629418___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain pushed 1 commit. cc8b9865b13bc06c96b404f12a9ea531e46ac4de Fix dynbrs with --quiet -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/e42c0a35a46bc99cd188c49e76f06cc424f2ba01..cc8b9865b13bc06c96b404f12a9ea531e46ac4de ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Found one more issue: --quiet also makes the BuildRequires disappear as the output of all build scripts is piped to /dev/null. OK, when exactly are those macro expanded...? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-495471368___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
As the GH interface doesn't allow commenting on individual commits, I'll just make a general comment... In the "Pass rpmts object to rpmSpecBuild()" commit, there are two minor issues: - a stray empty line added before buildSpec() - a change to rpmbuild.c that belongs to the next commit (move to library, return 11 on missing buildrequires) Other than that, and the missing tests, it's looking quite okay now. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-492961283___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -273,20 +287,6 @@ static struct poptOption optionsTable[] = {
POPT_TABLEEND
};
-static int checkSpec(rpmts ts, rpmSpec spec)
-{
-int rc;
-rpmps ps = rpmSpecCheckDeps(ts, spec);
-
-if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-rc = (ps != NULL);
-rpmpsFree(ps);
-return rc;
-}
-
Works for me.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r284579742___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > * @param spec spec file control structure * @param buildArgsbuild arguments - * @return RPMRC_OK on success + * @return 0 on success, 1 on build error, + * RPMRC_MISSINGBUILDREQUIRES on missing build The return values are bit of a mishmash, some numeral and some RPMRC_ value that's not really an RPMRC afterall. Would be more consistent to return RPMRC_* constants for everything (but keep return type as int because of RPMRC_MISSINGBUILDREQUIRES). That, or just talk about numbers. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-238228374___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +freeStringBuf(sb_stdout);
+free(output);
+return rc;
+}
+
+static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec spec, int test)
+{
+rpmRC rc = RPMRC_OK;
+rpmps ps = rpmSpecCheckDeps(ts, spec);
+
+if (ps) {
+ rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
+ rpmpsPrint(NULL, ps);
+}
+if (ps != NULL)
+ rc = RPMRC_MISSINGBUILDREQUIRES;
This latter (ps != NULL) is redundant, just move the rc setting to the if (ps)
case above. Sorry for missing on the previous rounds.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-238224478___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> (what & ~RPMBUILD_RMSOURCE) |
(x ? 0 : (what & RPMBUILD_PACKAGESOURCE) {
goto exit;
}
}
} else {
int didBuild = (what & (RPMBUILD_PREP|RPMBUILD_BUILD|RPMBUILD_INSTALL));
+ int sourceOnly = ((what & RPMBUILD_PACKAGESOURCE) &&
+ !(what & (RPMBUILD_BUILD|RPMBUILD_INSTALL|RPMBUILD_PACKAGEBINARY)));
+
+ if (!spec->buildrequires && sourceOnly){
There's a missing space between ) and { (clearly, getting to the bottom of
things...)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-238222866___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
OK, that should be the requested changes. Still missing is squashing of the three patches from "Pass rpmts object to rpmSpecBuild" onward and a test case or two. Patch set also get rebased to current head. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-492694889___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request.
> @@ -273,20 +287,6 @@ static struct poptOption optionsTable[] = {
POPT_TABLEEND
};
-static int checkSpec(rpmts ts, rpmSpec spec)
-{
-int rc;
-rpmps ps = rpmSpecCheckDeps(ts, spec);
-
-if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-rc = (ps != NULL);
-rpmpsFree(ps);
-return rc;
-}
-
Still kinda hesitant to just lump those patches together. But the move of the
build requires check is a separate patch now.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r284296944___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request. > @@ -16,6 +16,8 @@ #include "debug.h" +#define RPMRC_MISSINGBUILDREQUIRES 11 Oh, this acentially moved to the _internal header. Will move again... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#discussion_r284296301___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request.
> +
+if (!spec->buildrequires) {
+ rc = RPMRC_OK;
+ goto exit;
+}
+
+if ((rc = doScript(spec, RPMBUILD_BUILDREQUIRES, "%generate_buildrequires",
+ getStringBuf(spec->buildrequires), _stdout, test)))
+ goto exit;
+
+/* add results to requires of the srpm */
+argvSplit(, getStringBuf(sb_stdout), "\n\r");
+outc = argvCount(output);
+
+if (!outc) {
+ rc = RPMRC_OK;
Removed.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r284295417___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request. > @@ -47,7 +47,7 @@ static rpmRC doRmSource(rpmSpec spec) * @todo Single use by %%doc in files.c prevents static. */ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name, - const char *sb, int test) + const char *sb, StringBuf * sb_stdoutp, int test) Done. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#discussion_r284295068___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai requested changes on this pull request. As per above, changes requested. Hopefully this resets the review state properly, things have gotten a bit confused here (my bad) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-237715817___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
In the commit to pass rpmts object ro rpmSpecBuild() there's a stray empty line added to build/build.c, but it seems to get lost somewhere in the birds-eye view that GH considers review mode. The birds-eye view of things is useful for, well, a birds-eye view, but bulls*** for reviewing patch series *grumble* -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-491197242___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> if (rstreqn(N, "rpmlib(", sizeof("rpmlib(")-1)) {
- if (tagN != RPMTAG_REQUIRENAME) return 1;
+ if (tagN != RPMTAG_REQUIRENAME &&
+ (tagN == RPMTAG_PROVIDENAME && !(Flags & RPMSENSE_RPMLIB)))
+ return 1;
Fix the condition indentation.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235969658___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> rpmlog(RPMLOG_NOTICE, _("\n\nRPM build errors:\n"));
rpmlogPrint(NULL);
}
rpmugFree();
-
+if (missing_buildreqs && !rc) {
+ rc = RPMRC_MISSINGBUILDREQUIRES;
+}
+if (rc == RPMRC_FAIL)
+ rc = 1;
Please use consistent style with the to if's: either curly braces in both, or
neither.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235969374___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -273,20 +287,6 @@ static struct poptOption optionsTable[] = {
POPT_TABLEEND
};
-static int checkSpec(rpmts ts, rpmSpec spec)
-{
-int rc;
-rpmps ps = rpmSpecCheckDeps(ts, spec);
-
-if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-rc = (ps != NULL);
-rpmpsFree(ps);
-return rc;
-}
-
BTW that split also will also give a nice logical place to put the python
pass-the-ts changes into.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r282780416___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -273,20 +287,6 @@ static struct poptOption optionsTable[] = {
POPT_TABLEEND
};
-static int checkSpec(rpmts ts, rpmSpec spec)
-{
-int rc;
-rpmps ps = rpmSpecCheckDeps(ts, spec);
-
-if (ps) {
- rpmlog(RPMLOG_ERR, _("Failed build dependencies:\n"));
- rpmpsPrint(NULL, ps);
-}
-rc = (ps != NULL);
-rpmpsFree(ps);
-return rc;
-}
-
Please split this move of build-requires checking into librpmbuild with a new
special return code to a separate commit. It makes for a nice independent and
logical step that serves as a regression checkpoint against existing behavior
and also makes the final commit of the actual new feature more contained.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235967922___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > @@ -894,6 +895,9 @@ static rpmSpec parseSpec(const char *specFile, > rpmSpecFlags flags, case PART_PREP: parsePart = parsePrep(spec); break; + case PART_BUILDREQUIRES: + parsePart = parseSimpleScript(spec, "%generate_buildrequires", &(spec->buildrequires)); An overlong line, please split. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235965810___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > goto exit; + if (what & RPMBUILD_BUILDREQUIRES) +rc = doBuildRequires(spec, test); + if ((what & RPMBUILD_CHECKBUILDREQUIRES) && + (rc == RPMRC_MISSINGBUILDREQUIRES)) + rc = doCheckBuildRequires(ts, spec, test); The condition and if-block are on the same indentation level here, please fix. This whole doScript() related if-section is quite a mess to begin with and violates rpm general style, so an alternative to indenting the conditional is indenting the if-block deeper, as is done with the goto exit's below. Readability is far more important than following style to the letter, and consistency with surroundings helps readability too. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235965472___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +/* add results to requires of the srpm */
+argvSplit(, getStringBuf(sb_stdout), "\n\r");
+outc = argvCount(output);
+
+if (!outc) {
+ rc = RPMRC_OK;
+ goto exit;
+}
+
+for (int i = 0; i < outc; i++) {
+ parseRCPOT(spec, spec->sourcePackage, output[i], RPMTAG_REQUIRENAME, 0,
0, addReqProvPkg, NULL);
+}
+
+rpmdsPutToHeader(*packageDependencies(spec->sourcePackage,
RPMTAG_REQUIRENAME), spec->sourcePackage->header);
+
+parseRCPOT(spec, spec->sourcePackage, "rpmlib(DynamicBuildRequires) =
4.15.0-1", RPMTAG_PROVIDENAME, 0, RPMSENSE_FIND_PROVIDES | RPMSENSE_RPMLIB,
addReqProvPkg, NULL);
There are some seriously overlong lines here, both parseRCPOT() calls and the
rpmdsPutToHeader(). Please adjust to 80 character line-length.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235955768___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -213,24 +278,58 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec,
> int what)
} else {
int didBuild = (what & (RPMBUILD_PREP|RPMBUILD_BUILD|RPMBUILD_INSTALL));
+ if (!spec->buildrequires && (what & RPMBUILD_PACKAGESOURCE) &&
+ !(what & (RPMBUILD_BUILD|RPMBUILD_INSTALL|RPMBUILD_PACKAGEBINARY))){
This would far easier to read if at least part of it was split into a helper
variable, similar to didBuild above.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235953811___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > @@ -16,6 +16,8 @@ #include "debug.h" +#define RPMRC_MISSINGBUILDREQUIRES 11 Hmm, we're actually returning this in the API so I think it should actually go into rpmbuild.h -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235954708___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +for (int i = 0; i < outc; i++) {
+ parseRCPOT(spec, spec->sourcePackage, output[i], RPMTAG_REQUIRENAME, 0,
0, addReqProvPkg, NULL);
+}
+
+rpmdsPutToHeader(*packageDependencies(spec->sourcePackage,
RPMTAG_REQUIRENAME), spec->sourcePackage->header);
+
+parseRCPOT(spec, spec->sourcePackage, "rpmlib(DynamicBuildRequires) =
4.15.0-1", RPMTAG_PROVIDENAME, 0, RPMSENSE_FIND_PROVIDES | RPMSENSE_RPMLIB,
addReqProvPkg, NULL);
+rc = RPMRC_MISSINGBUILDREQUIRES;
+
+ exit:
+freeStringBuf(sb_stdout);
+free(output);
+return rc;
+}
+
+static rpmRC doCheckBuildRequires(rpmts ts, rpmSpec spec, int test)
This too should be an int, because RPMRC_MISSINGBUILDREQUIRES is not a valid
value of rpmRC.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235953324___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -172,9 +179,67 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const
> char *name,
return rc;
}
-static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int what)
+static rpmRC doBuildRequires(rpmSpec spec, int test)
+{
+StringBuf sb_stdout = NULL;
+int outc;
+ARGV_t output = NULL;
+
+rpmRC rc = 1; /* assume failure */
Use an int for the type here and for the function return code, since that's
what we're really returning.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235952294___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +
+if (!spec->buildrequires) {
+ rc = RPMRC_OK;
+ goto exit;
+}
+
+if ((rc = doScript(spec, RPMBUILD_BUILDREQUIRES, "%generate_buildrequires",
+ getStringBuf(spec->buildrequires), _stdout, test)))
+ goto exit;
+
+/* add results to requires of the srpm */
+argvSplit(, getStringBuf(sb_stdout), "\n\r");
+outc = argvCount(output);
+
+if (!outc) {
+ rc = RPMRC_OK;
This is assignment is redundant, rc is already ok if we get this far.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235951890___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > @@ -283,7 +284,7 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int > what) return rc; } -rpmRC rpmSpecBuild(rpmSpec spec, BTA_t buildArgs) +int rpmSpecBuild(rpmts ts, rpmSpec spec, BTA_t buildArgs) Just noting that this is an API break. We're breaking API in some other places too in this cycle so that's okay, just saying it aloud to remind myself basically. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235949772___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > @@ -172,6 +172,7 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const > char *name, return rc; } + Stray newline added. Like said in an earlier comment, the addition of rpmtsFromPyObject() could be merged with this commit because it doesn't make much sense alone, especially without any rationale in the commit message. And here too: explain why this is added in the message. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235949222___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request. > @@ -47,7 +47,7 @@ static rpmRC doRmSource(rpmSpec spec) * @todo Single use by %%doc in files.c prevents static. */ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const char *name, - const char *sb, int test) + const char *sb, StringBuf * sb_stdoutp, int test) Return value arguments should generally be the last. We failed to adjust rpmfcExec() accordingly but perhaps we could still fix this one... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235946561___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Argh, I didn't mean to approve the whole thing, thought I could do it for the couple of earlier commits separately. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-491175085___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-235945251___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
RPM does not consider provides of SRPM for dependency check. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-490965693___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
OK, here is the rebased and squashed patch set. Let's hope not too many things broke during this process... I noticed that we are using the same rpmlib name for indicating that the SRPM has an %generate_buildrequires script (and though needs support for this feature) and that it already contains the results. I've not yet tested this. But I worry about the srpm using this asa self provide - which is probably bad. I'll add some test cases tomorrow. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-490963460___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Ah yup, include the doScript() changes too. Redoing the series with fixes included is usually not that much effort when you have the whole thing working, just take the resulting big diff and commit necessary infra changes first. Most of this belongs into a single commit to add the finished feature. Oh and do add at least a rudimentary test-case or two for the feature. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-489961658___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
I'll fix the cosmetics. I also have patches to use doScript() for the running the buildRequires script that removes a lot of duplicated code but have not made it into this PR yet. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-489637459___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Please clean up the recurring cosmetic issues mentioned above, and redo the series to include later fixes from the start so we get a nice, logical patch series for proper review. Adding rpmtsFromPyObject() can be merged in the commit to pass rpmts to spec build, because it's a small thing really just part of the same change. Ditto with making freeSources() available and actually using, but then that goes away when the series is redone. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-489634594___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -274,17 +414,22 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec,
> int what)
exit:
free(cookie);
spec->rootDir = NULL;
-if (rc != RPMRC_OK && rpmlogGetNrecs() > 0) {
+if (rc != RPMRC_OK && rc != RPMRC_MISSINGBUILDREQUIRES &&
+ rpmlogGetNrecs() > 0) {
rpmlog(RPMLOG_NOTICE, _("\n\nRPM build errors:\n"));
The if-condition continues at the same indentation level as the actual if-block
itself, so its really easy to mistake the rpmlogGetNrecs() to be part of the
if-block rather than the condition. This is a recurring theme in the patches,
please fix all such cases. Multiple options how to deal with it:
a) indent the continued condition to clearly deeper level (iirc this is what
```indent -kr``` does)
b) find a way to make the condition fit on one line, using helper variables and
such as necessary
c) put the opening curly brace on a line of its own - its a kind of style
violation but it does make the construct obvious
With meaningful names for helper variables, b) typically produces by the most
readable code.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-233993450___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> @@ -172,9 +174,113 @@ rpmRC doScript(rpmSpec spec, rpmBuildFlags what, const
> char *name,
return rc;
}
-static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int what)
+static rpmRC doBuildRequires(rpmSpec spec, int test) {
In case of functions, the opening curly brace belongs to a line of its own.
This is a recurring theme, please check and fix all newly introduced functions.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-233985799___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@pmatilai I think this is ready to go. could you review it, please? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-487850625___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
So this is passing now. :champagne: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-487371014___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain pushed 1 commit. 3ec53ad49a5b002e71d9aed97938e50ffb5eb9fa fixup! Add support for dynamic BuildRequires -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/58d2997e0834add1790927a134584ba6853dd43d..3ec53ad49a5b002e71d9aed97938e50ffb5eb9fa ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
I have rebased this on top of https://github.com/rpm-software-management/rpm/pull/686 so that one needs to go first. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-487370353___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Cool, I'll send patch. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-486604885___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> If you are talking about having pythonX-rpm to depend on rpm-build-libs and > rpm-sign-libs, but keeping C libraries separate, that would make sense to me > (since it does not affect minimal installations). Yup, that's exactly what I mean. The python side was split to keep in spirit of the C-side split and to avoid dragging all those build/sign dependencies into minimal roots, but then Python and minimal ... don't really fit into the same sentence. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-486603334___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> I think a better way would be just merging the python modules back into one > instead of splitting by C-side library, the split causes way more problems > than it solves. If you are talking about having pythonX-rpm to depend on rpm-build-libs and rpm-sign-libs, but keeping C libraries separate, that would make sense to me (since it does not affect minimal installations). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-486541897___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> I would vote for removing _doBuild method entirely since it is private so > nobody should use it anyway. I think a better way would be just merging the python modules back into one instead of splitting by C-side library, the split causes way more problems than it solves. > You need an empty build section to get it defined properly, I think. %build section presence is what *enables* the debuginfo stuff. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-486539688___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@nim-nim if you have bootstrap enabled then yes, different dnf is being used. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-485233424___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@xsuchy: that was I thought too but the first install report in the log does not have the problem. Unless mock uses a different dnf for different build stages? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-485065348___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
You mean that 'noarch1.3.0-1.fc3'? But that is printed by DNF. You can run 'mock -v' to see what mock pass to DNF. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484977714___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@xsuchy the glued `ArchVersion` (without separator) in package install reports seems like a mock bug -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484922747___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
```
INFO: mock.py version 1.4.14 starting (python version = 3.7.3)...
Start: init plugins
[…]
Finish: chroot init
Start: build phase for golang-github-sirupsen-logrus-1.1.0-1.0.fc31.src.rpm
Start: build setup for golang-github-sirupsen-logrus-1.1.0-1.0.fc31.src.rpm
Building target platforms: x86_64
Building for target x86_64
Wrote:
/builddir/build/SRPMS/golang-github-sirupsen-logrus-1.1.0-1.0.fc31.mng.src.rpm
/usr/lib/python3.7/site-packages/mockbuild/util.py:386: UnicodeWarning:
decode() called on unicode string, see
https://bugzilla.redhat.com/show_bug.cgi?id=1693751
return tuple(x.decode() if i != 1 else x for i, x in enumerate(ret))
fedora-macros-ng1.5 MB/s | 1.5 kB 00:00
fedora 27 kB/s | 19 kB 00:00
Copr repo for mock owned by @mock 6.2 kB/s | 3.5 kB 00:00
Copr repo for rpm-buildreqs owned by ignatenkob 6.2 kB/s | 3.5 kB 00:00
local 8.8 kB/s | 3.8 kB 00:00
Dependencies resolved.
Package Arch Version Repository Size
Installing:
go-rpm-macros x86_64 3.0.4-0.34.0.20190419git9c07ff3.fc31.mng
rawhide-mng 34 k
Installing dependencies:
go-filesystem x86_64 3.0.4-0.34.0.20190419git9c07ff3.fc31.mng
rawhide-mng 6.6 k
golang x86_64 1.12.2-1.0.fc31.mng.1+20181202pr1 rawhide-mng 630 k
[…]
Complete!
Finish: build setup for golang-github-sirupsen-logrus-1.1.0-1.0.fc31.src.rpm
Start: rpmbuild golang-github-sirupsen-logrus-1.1.0-1.0.fc31.src.rpm
Start: Outputting list of installed packages
Finish: Outputting list of installed packages
Building target platforms: x86_64
Building for target x86_64
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.Ymyt6v
[…]
+ exit 0
Executing(buildreqs): /bin/sh -e /var/tmp/rpm-tmp.HBp9vv
+ umask 022
+ cd /builddir/build/BUILD
+ cd /builddir/build/BUILD
+ cd logrus-1.1.0
++ GOPATH=/builddir/build/BUILD/logrus-1.1.0/_build:/usr/share/gocode
++ golist --imported --package-path github.com/sirupsen/logrus --skip-self
++ GOPATH=/builddir/build/BUILD/logrus-1.1.0/_build:/usr/share/gocode
++ golist --imported --package-path github.com/sirupsen/logrus --skip-self
--tests
+ sort -u
+ xargs '-I{}' echo 'golang({})'
+ exit 0
Wrote:
/builddir/build/SRPMS/golang-github-sirupsen-logrus-1.1.0-1.0.fc31.mng.buildreqs.nosrc.rpm
INFO: Dynamic buildrequires detected
INFO: Going to install missing buildrequires
fedora-macros-ng1.5 MB/s | 1.5 kB 00:00
fedora 106 kB/s | 19 kB 00:00
Copr repo for mock owned by @mock 6.3 kB/s | 3.5 kB 00:00
Copr repo for rpm-buildreqs owned by ignatenkob 6.3 kB/s | 3.5 kB 00:00
local 8.5 kB/s | 3.8 kB 00:00
Package go-rpm-macros-3.0.4-0.34.0.20190419git9c07ff3.fc31.mng.x86_64 is
already installed.
Dependencies resolved.
Package ArchVersion
Repository Size
Installing:
golang-github-golang-sys-devel x86_640-0.24.20190302gitb688937.fc31
fedora 97 k
golang-github-stretchr-testify-devel noarch1.3.0-1.fc30
fedora 64 k
golang-golangorg-crypto-develx86_640-0.29.20190324gitb7391e9.fc31
fedora 984 k
Installing dependencies:
golang-github-davecgh-spew-devel noarch1.1.1-3.0.fc31.mng
rawhide-mng 41 k
golang-github-pmezard-go-difflib-devel noarch0-0.14.20180621git792786c.fc30
fedora 21 k
golang-github-stretchr-objx-develnoarch0-0.15.git1a9d0bb.fc30
fedora 24 k
Transaction Summary
Install 6 Packages
[…]
Complete!
Building target platforms: x86_64
Building for target x86_64
Executing(buildreqs): /bin/sh -e /var/tmp/rpm-tmp.SP3Tnt
+ umask 022
+ cd /builddir/build/BUILD
+ cd /builddir/build/BUILD
+ cd logrus-1.1.0
++ GOPATH=/builddir/build/BUILD/logrus-1.1.0/_build:/usr/share/gocode
++ golist --imported --package-path github.com/sirupsen/logrus --skip-self
++ GOPATH=/builddir/build/BUILD/logrus-1.1.0/_build:/usr/share/gocode
++ golist --imported --package-path github.com/sirupsen/logrus --skip-self
--tests
+ sort -u
+ xargs '-I{}' echo 'golang({})'
+ exit 0
Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.OZpQ3u
[…]
Checking for unpackaged file(s): /usr/lib/rpm/check-files
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@Conan-Kudo empty `%build` didn’t work so in the past, you needed no `%build` section at all. If someone changed the logic to empty `%build`, that's gonna break loads of specs -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484917919___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
You need an empty build section to get out defined properly, I think. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484917290___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Ok, with `rpm-4.14.2.1-7.fc31+buildreqs.1.x86_64` and `mock-1.4.14-1.git.34.3723a7c.fc31.noarch` I see the whole build chain working. *However* something seems to have broken debug generation logic, I get the dreaded ``` Empty %files file /builddir/build/BUILD/logrus-1.1.0/debugsourcefiles.list ``` even though the spec does not include any `%build` section -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484913184___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> The problem is that the function lives in an entirely different DSO (_rpm.so) > which is not linked in by the build module (_rpmb.so) I spent 30 minutes googling and trying things and I could not find way how to link it. --- I would vote for removing `_doBuild` method entirely since it is private so nobody should use it anyway. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484850727___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
This is what's wrong with rpmtsFromPyObject(): ``` /usr/bin/ld: .libs/spec-py.o: in function `spec_doBuild': /home/pmatilai/repos/rpm/python/spec-py.c:303: undefined reference to `rpmtsFromPyObject' collect2: error: ld returned 1 exit status ``` The problem is that the function lives in an entirely different DSO (_rpm.so) which is not linked in by the build module (_rpmb.so). The per-library split is a major PITA and something we're increasingly running into. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484464988___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> Yes, building rpm-python I sing system library as I reported this few years > ago .. it was not fixed. Well, you kinda promised to fix it yourself in here: https://github.com/rpm-software-management/rpm/issues/130#issuecomment-273552868 ;) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484454522___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request.
> @@ -237,11 +343,45 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec,
> int what)
} else {
int didBuild = (what & (RPMBUILD_PREP|RPMBUILD_BUILD|RPMBUILD_INSTALL));
+ if (!spec->buildrequires && (what & RPMBUILD_PACKAGESOURCE) &&
+ !(what & (RPMBUILD_BUILD|RPMBUILD_INSTALL|RPMBUILD_PACKAGEBINARY))){
+ /* don't run prep if not needed for source build */
+ /* with(out) dynamic build requires*/
+ what &= ~(RPMBUILD_PREP);
+ }
+
+ if ((what & RPMBUILD_CHECKBUILDREQUIRES) &&
Well, these dependencies may be needed during %prep or (more likely) during
%generate_buildrequires. We don't want these to just blow up. The later check
makes sure the newly created buildrequires are there.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r276560780___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Yes, building rpm-python I sing system library as I reported this few years ago .. it was not fixed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484393412___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ignatenkobrain commented on this pull request.
> @@ -237,11 +343,45 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec,
> int what)
} else {
int didBuild = (what & (RPMBUILD_PREP|RPMBUILD_BUILD|RPMBUILD_INSTALL));
+ if (!spec->buildrequires && (what & RPMBUILD_PACKAGESOURCE) &&
+ !(what & (RPMBUILD_BUILD|RPMBUILD_INSTALL|RPMBUILD_PACKAGEBINARY))){
+ /* don't run prep if not needed for source build */
+ /* with(out) dynamic build requires*/
+ what &= ~(RPMBUILD_PREP);
+ }
+
+ if ((what & RPMBUILD_CHECKBUILDREQUIRES) &&
Do we need to check it here? Given that we check it few lines later.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-228141464___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
8108cc7aa9735f629d7380f94690d2947abeb9db still trips up test 135. My guess is there is something wrong with the test suite. Probably linking the Python bindings to the outside (system) rpmlib. It passes if I don't access the new rpmtsFromPyObject() function. The import also works when run on the libs in the python/ subdir. I will investigate this further but I am not willing to delay review till then. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-484392946___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ffesti pushed 4 commits. 3feac842c7a4fbfb1fde68ed5c15a389fb9ec941 Add rpmtsFromPyObject 8108cc7aa9735f629d7380f94690d2947abeb9db Pass rpmts object to rpmSpecBuild() 346e561a0be8366290026bdfd360983e36a834e8 Do build dependency check in build/build.c instead of rpmbuild 835f31c2f125d324121bb2bc27967d2fe82c7d62 Always rebuild the test-suite testing-environment from scratch -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/7d339dff7a69c778bdef4bdbae16e52ed20ac36a..835f31c2f125d324121bb2bc27967d2fe82c7d62 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
For the record - related changes in redhat-rpm-config https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/51 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-483148006___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain The example in https://fedoraproject.org/wiki/Changes/DynamicBuildRequires?rd=Changes/BuildRequires_Generators#Example is too complicated. I would suggest to put there some artificial example which add one artificial dependency and it will use just cat/echo and everything (including the macro definition) will be defined in that example. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-483147410___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain `rpm-build-4.14.2.1-7.fc31+buildreqs.1.x86_64` works for me:
```console
$ grep BuildRequires ~/fedora/rpm/golist/golist.spec
$ LANG=C.utf8 rpmbuild -ba ~/fedora/rpm/golist/golist.spec
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.t0kf16
+ umask 022
+ cd /var/lib/builder/rpmbuild/BUILD
[…]
Executing(buildreqs): /bin/sh -e /var/tmp/rpm-tmp.sOlg54
+ umask 022
+ cd /var/lib/builder/rpmbuild/BUILD
+ cd /var/lib/builder/rpmbuild/BUILD
+ cd golist-263dcfff17382fb43c241ae46a555f8072576526
++
GOPATH=/var/lib/builder/rpmbuild/BUILD/golist-263dcfff17382fb43c241ae46a555f8072576526/_build:/usr/share/gocode
++ golist --imported --package-path pagure.io/golist --skip-self
++
GOPATH=/var/lib/builder/rpmbuild/BUILD/golist-263dcfff17382fb43c241ae46a555f8072576526/_build:/usr/share/gocode
++ golist --imported --package-path pagure.io/golist --skip-self --tests
+ sort -u
+ xargs '-I{}' echo 'golang({})'
+ exit 0
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.wgrBm6
+ umask 022
+ cd /var/lib/builder/rpmbuild/BUILD
+ cd golist-263dcfff17382fb43c241ae46a555f8072576526
[…]
Wrote:
/var/lib/builder/rpmbuild/SRPMS/golist-0.9.1-0.5.20190410git263dcff.fc31.src.rpm
Wrote:
/var/lib/builder/rpmbuild/RPMS/x86_64/golist-0.9.1-0.5.20190410git263dcff.fc31.x86_64.rpm
Wrote:
/var/lib/builder/rpmbuild/RPMS/noarch/golang-pagure-golist-devel-0.9.1-0.5.20190410git263dcff.fc31.noarch.rpm
Wrote:
/var/lib/builder/rpmbuild/RPMS/x86_64/golist-debugsource-0.9.1-0.5.20190410git263dcff.fc31.x86_64.rpm
Wrote:
/var/lib/builder/rpmbuild/RPMS/x86_64/golist-debuginfo-0.9.1-0.5.20190410git263dcff.fc31.x86_64.rpm
Executing(%clean): /bin/sh -e /var/tmp/rpm-tmp.BuGRl7
+ umask 022
+ cd /var/lib/builder/rpmbuild/BUILD
+ cd golist-263dcfff17382fb43c241ae46a555f8072576526
+ /usr/bin/rm -rf
/var/lib/builder/rpmbuild/BUILDROOT/golist-0.9.1-0.5.20190410git263dcff.fc31.x86_64
+ exit 0
$ rpm -qp --requires
/var/lib/builder/rpmbuild/SRPMS/golist-0.9.1-0.5.20190410git263dcff.fc31.src.rpm
go-rpm-macros
golang(github.com/urfave/cli)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(DynamicBuildRequires) <= 4.15.0-1
rpmlib(FileDigests) <= 4.6.0-1
```
@xsuchy I couldn't locate a pre-built mock to test with, and I'm out of time,
so that will be all for today.
The tested spec is
[here](https://copr-be.cloud.fedoraproject.org/results/nim/macros-ng/fedora-rawhide-x86_64/00881747-golist/golist.spec).
It needs the rest of the
[copr](https://copr.fedorainfracloud.org/coprs/nim/macros-ng/). (And, BTW, I’m
real sick of carrying the remainder of [redhat-rpm-config
PR51](https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/51)).
Unless I made a mistake somewhere, with this copr all Fedora golang packages
that do not inhibit a BuildRequires or force it to a specific version, should
rebuild by:
* removing the following lines:
```rpm
BuildRequires: golang(foo)
```
* and adding
```rpm
%generate_buildrequires
%go_generate_buildrequires
```
And now I’ll switch to finishing the macro glue for my Go module dynamic
BuildRequires generator, so I can test it too. Unlike this generator it will
handle complex upstream versioning constrains and should require less shell
massaging.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#issuecomment-483029345___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain pushed 3 commits. 19961172f5ebc3d728eeeb6463d6478a3278bd59 rpmbuild: Generate buildreqs.nosrc.rpm properly with --nodeps c2f5358a3346eb8b886d87aeb2b66605094aa19a rpmbuild: Put dynamic BuildRequires into src.rpm even with --nodeps 7d339dff7a69c778bdef4bdbae16e52ed20ac36a Fix providing of rpmlib(DynamicBuildRequires) in src.rpm -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/93e5e3e408caf4d94eeb4acf412a8ca341cb93df..7d339dff7a69c778bdef4bdbae16e52ed20ac36a ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> @ignatenkobrain can you please send me some spec/src.rpm with dynamic > dependencies? @xsuchy you have another one here https://copr.fedorainfracloud.org/coprs/nim/macros-ng/build/881699/ (needs the rest of the copr, and obviously failing to build in copr) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481838675___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain your releases are not big enough to be picked up over the rpm version in koji rawhide Koji rawhide is already at release 6 https://koji.fedoraproject.org/koji/packageinfo?packageID=319 Also, the modified rpm seems to break some copr assumptions * with ` copr://ignatenkobrain/rpm-buildreqs ` enabled: https://copr.fedorainfracloud.org/coprs/nim/macros-ng/build/881540/ * without it, same srpm upload: https://copr.fedorainfracloud.org/coprs/nim/macros-ng/build/881542/ -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481689767___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ffesti pushed 2 commits. a27f82f955b6f09d5f44fa7627f7db2f8b867d81 fixup! Add support for dynamic BuildRequires 93e5e3e408caf4d94eeb4acf412a8ca341cb93df fixup! Add support for dynamic BuildRequires -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/441e835149fb71640d3a43458c925bfc533fea47..93e5e3e408caf4d94eeb4acf412a8ca341cb93df ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> Thinking about this a bit more, rpmbuild should probably get an option to > just execute genbr section without checking dependencies. > @ffesti thoughts? The thing is that I had (and in the code still have) the execution of the br script separate from the dependency check. But this makes the assumption that the br script only needs to run once. The way it is done now the br script can write out its own dependencies and then give up and continue the work the next time around. This allows using only the very minimal static build requires. If we assume the br script is only running once several things can be made more efficient. E.g. rpmbuild -br only needs the static build requires. But it requires more work on the packager's side - which is what we want to avoid in the first place. Code wise it is easy to implement it either way. But we have to choose one option and live with the consequences. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481643070___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Just some update on this from my side:
* RPM with this PR
* Latest mock
** With https://github.com/rpm-software-management/mock/pull/246
** With removed `--nodeps` from mock (@xsuchy is going to test whether it will
break anything)
* rust-packaging with https://pagure.io/fedora-rust/rust2rpm/pull-request/75
I'm taking `rust-warp` from fedora dist-git, apply following diff and running
mock.
```diff
diff --git a/rust-warp.spec b/rust-warp.spec
index ba41816..a6af11c 100644
--- a/rust-warp.spec
+++ b/rust-warp.spec
@@ -16,28 +16,6 @@ Source: %{crates_source}
ExclusiveArch: %{rust_arches}
BuildRequires: rust-packaging
-BuildRequires: (crate(bytes/default) >= 0.4.8 with crate(bytes/default) <
0.5.0)
-BuildRequires: (crate(futures/default) >= 0.1.0 with crate(futures/default) <
0.2.0)
-BuildRequires: (crate(headers/default) >= 0.2.0 with crate(headers/default) <
0.3.0)
-BuildRequires: (crate(http/default) >= 0.1.0 with crate(http/default) < 0.2.0)
-BuildRequires: (crate(hyper/default) >= 0.12.18 with crate(hyper/default) <
0.13.0)
-BuildRequires: (crate(log/default) >= 0.4.0 with crate(log/default) < 0.5.0)
-BuildRequires: (crate(mime/default) >= 0.3.0 with crate(mime/default) < 0.4.0)
-BuildRequires: (crate(mime_guess/default) >= 2.0.0~alpha.6 with
crate(mime_guess/default) < 3.0.0)
-BuildRequires: (crate(scoped-tls/default) >= 1.0.0 with
crate(scoped-tls/default) < 2.0.0)
-BuildRequires: (crate(serde/default) >= 1.0.0 with crate(serde/default) <
2.0.0)
-BuildRequires: (crate(serde_json/default) >= 1.0.0 with
crate(serde_json/default) < 2.0.0)
-BuildRequires: (crate(serde_urlencoded/default) >= 0.5.3 with
crate(serde_urlencoded/default) < 0.6.0)
-BuildRequires: (crate(tokio-io/default) >= 0.1.0 with crate(tokio-io/default)
< 0.2.0)
-BuildRequires: (crate(tokio-threadpool/default) >= 0.1.7 with
crate(tokio-threadpool/default) < 0.2.0)
-BuildRequires: (crate(tokio/default) >= 0.1.11 with crate(tokio/default) <
0.2.0)
-BuildRequires: (crate(tungstenite) >= 0.6.0 with crate(tungstenite) < 0.7.0)
-BuildRequires: (crate(urlencoding/default) >= 1.0.0 with
crate(urlencoding/default) < 2.0.0)
-%if %{with check}
-BuildRequires: (crate(handlebars/default) >= 1.0.0 with
crate(handlebars/default) < 2.0.0)
-BuildRequires: (crate(pretty_env_logger/default) >= 0.3.0 with
crate(pretty_env_logger/default) < 0.4.0)
-BuildRequires: (crate(serde_derive/default) >= 1.0.0 with
crate(serde_derive/default) < 2.0.0)
-%endif
%global _description \
Serve the web at warp speeds.
@@ -74,6 +52,9 @@ which use "default" feature of "%{crate}" crate.
%autosetup -n %{crate}-%{version_no_tilde} -p1
%cargo_prep
+%generate_buildrequires
+%cargo_generate_buildrequires
+
%build
%cargo_build
```
```
Start: rpmbuild rust-warp-0.1.15-1.fc31.src.rpm
…
Executing(buildreqs): /bin/sh -e /var/tmp/rpm-tmp.Ifckju
+ umask 022
+ cd /builddir/build/BUILD
+ cd /builddir/build/BUILD
+ cd warp-0.1.15
+ /usr/bin/cargo-inspector -BR Cargo.toml
+ exit 0
error: Failed build dependencies:
(crate(bytes/default) >= 0.4.8 with crate(bytes/default) < 0.5.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(futures/default) >= 0.1.0 with crate(futures/default) < 0.2.0)
is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(headers/default) >= 0.2.0 with crate(headers/default) < 0.3.0)
is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(http/default) >= 0.1.0 with crate(http/default) < 0.2.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(hyper/default) >= 0.12.18 with crate(hyper/default) < 0.13.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(log/default) >= 0.4.0 with crate(log/default) < 0.5.0) is needed
by rust-warp-0.1.15-1.fc31.x86_64
(crate(mime/default) >= 0.3.0 with crate(mime/default) < 0.4.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(mime_guess/default) >= 2.0.0~alpha.6 with
crate(mime_guess/default) < 3.0.0) is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(scoped-tls/default) >= 1.0.0 with crate(scoped-tls/default) <
2.0.0) is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(serde/default) >= 1.0.0 with crate(serde/default) < 2.0.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(serde_json/default) >= 1.0.0 with crate(serde_json/default) <
2.0.0) is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(serde_urlencoded/default) >= 0.5.3 with
crate(serde_urlencoded/default) < 0.6.0) is needed by
rust-warp-0.1.15-1.fc31.x86_64
(crate(tokio-io/default) >= 0.1.0 with crate(tokio-io/default) < 0.2.0)
is needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(tokio-threadpool/default) >= 0.1.7 with
crate(tokio-threadpool/default) < 0.2.0) is needed by
rust-warp-0.1.15-1.fc31.x86_64
(crate(tokio/default) >= 0.1.11 with crate(tokio/default) < 0.2.0) is
needed by rust-warp-0.1.15-1.fc31.x86_64
(crate(tungstenite) >= 0.6.0 with crate(tungstenite) < 0.7.0) is
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
ffesti commented on this pull request.
> @@ -350,6 +350,18 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec,
> int what)
if ((what & RPMBUILD_CHECKBUILDREQUIRES) &&
(rc = doCheckBuildRequires(spec, test))) {
if (rc == RPMRC_MISSINGBUILDREQUIRES) {
+ /* Create buildreqs package */
+ char *nvr = headerGetAsString(spec->packages->header,
RPMTAG_NVR);
+ rasprintf(>sourceRpmName, "%s.buildreqs.rpm", nvr);
+ free(nvr);
+ /* free sources to not include them in the buildreqs package */
+ while (spec->sources) {
+ struct Source *t = spec->sources;
+ spec->sources = t->next;
+ _free(t->fullSource);
+ free(t);
+ }
Done.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r273895638___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ffesti pushed 5 commits. b2bb450b8b9a8548d10b8e6a3911f73f0ea9211d fixup! Add support for dynamic BuildRequires 3a8b02d637a1c083463357fdb3b6c8cb5273813c fixup! rpmbuild: Allow to build source with dymamic build dependencies included 23fcd00c01aca50ff00588106c23a7e764ca2a8b Use buildreqs.nosrc.rpm suffix cdfe6073e3a5af83594dee15a302b0f70bf56715 Make freeSources() available internally 441e835149fb71640d3a43458c925bfc533fea47 Use freeSources() instead of manually freeing the entries -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/593/files/30fed42b2a9eaeb9970fa6b4bc32f416027c7189..441e835149fb71640d3a43458c925bfc533fea47 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Based on the comment there, the idea there is to set RPMBUILD_PREP bit to zero, |= is certainly wrong for that purpose (but I dont know if there's something else wrong in the surrounding logic). However the general rpm-style for disabling bits would is: ``` what &= ~RPMBUILD_PREP; ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481620129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ffesti you have one more bug which is breaking basically everything :)
```diff
@@ -354,7 +353,7 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, int
what)
!(what & (RPMBUILD_BUILD|RPMBUILD_INSTALL|RPMBUILD_PACKAGEBINARY))){
/* don't run prep if not needed for source build */
/* with(out) dynamic build requires*/
- what &= !RPMBUILD_PREP;
+ what |= !RPMBUILD_PREP;
}
if ((what & RPMBUILD_PREP) &&
```
I hope I understood intention correctly
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481616585___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Lets discuss Mock implementation here: https://github.com/rpm-software-management/mock/issues/245 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481599005___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Thinking about this a bit more, rpmbuild should probably get an option to just execute genbr section without checking dependencies. The reason why mock does `--nodeps` because rpmbuild in target environment might be older/compiled with different features so it can't read rpmdb within chroot (because it was populated by other RPM). So looking at the code, I think: * `POPT_BR` should set just `RPMBUILD_BUILDREQUIRES` (even when `noDeps` is set so that it ignores BuildRequires from spec, but still does generate additional BuildRequires) * `RPMBUILD_BUILDREQUIRES` should write `buildreqs.nosrc.rpm` (not `RPMBUILD_CHECKBUILDREQUIRES`) * It should do only in case there is `%generate_buildrequires` section (writing it always means that all tooling will be slower because every time it would need to run builddep on it) @ffesti thoughts? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481540550___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
I have used standard `fedora-rawhide-x86_64.cfg` with 2 additional repos: ```ini [ignatenkobrain-rpm-buildreqs] name=Copr repo for rpm-buildreqs owned by ignatenkobrain baseurl=https://copr-be.cloud.fedoraproject.org/results/ignatenkobrain/rpm-buildreqs/fedora-rawhide-$basearch/ type=rpm-md skip_if_unavailable=False gpgcheck=1 gpgkey=https://copr-be.cloud.fedoraproject.org/results/ignatenkobrain/rpm-buildreqs/pubkey.gpg repo_gpgcheck=0 enabled=1 enabled_metadata=1 [ignatenkobrain-rpm-buildreqs-2] baseurl=https://ignatenkobrain.fedorapeople.org/buildreqs/ type=rpm-md skip_if_unavailable=True gpgcheck=0 repo_gpgcheck=0 enabled=1 ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481450364___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
Seems that it doesn't quite work (not sure why yet):
```
INFO: ENTER ['do'](['bash', '--login', '-c', '/usr/bin/rpmbuild -bb --target
x86_64 --nodeps /builddir/build/SPECS/rust-warp.spec'],
chrootPath='/var/lib/mock/fedora-rawhide-x86_64-buildreqs/root'env={'TERM':
'vt100', 'SHELL': '/bin/bash', 'HOME': '/builddir', 'HOSTNAME': 'mock', 'PATH':
'/usr/bin:/bin:/usr/sbin:/sbin', 'PROMPT_COMMAND': 'printf
"\\033]0;\\007"', 'PS1': ' \\s-\\v\\$ ', 'LANG':
'en_US.UTF-8'}shell=Falselogger=timeout=0uid=1000gid=135user='mockbuild'nspawn_args=['--capability=cap_ipc_lock',
'--bind=/tmp/mock-resolv.9ce04myj:/etc/resolv.conf']unshare_net=TrueprintOutput=True)
DEBUG: child environment: None
DEBUG: Executing command: ['/usr/bin/systemd-nspawn', '-q', '-M',
'0e1a55f2c21348f683147f4abb2a2339', '-D',
'/var/lib/mock/fedora-rawhide-x86_64-buildreqs/root', '-a',
'--capability=cap_ipc_lock',
'--bind=/tmp/mock-resolv.9ce04myj:/etc/resolv.conf', '--setenv=TERM=vt100',
'--setenv=SHELL=/bin/bash', '--setenv=HOME=/builddir',
'--setenv=HOSTNAME=mock', '--setenv=PATH=/usr/bin:/bin:/usr/sbin:/sbin',
'--setenv=PROMPT_COMMAND=printf "\\033]0;\\007"',
'--setenv=PS1= \\s-\\v\\$ ', '--setenv=LANG=en_US.UTF-8', '-u',
'mockbuild', 'bash', '--login', '-c', '/usr/bin/rpmbuild -bb --target x86_64
--nodeps /builddir/build/SPECS/rust-warp.spec'] with env {'TERM': 'vt100',
'SHELL': '/bin/bash', 'HOME': '/builddir', 'HOSTNAME': 'mock', 'PATH':
'/usr/bin:/bin:/usr/sbin:/sbin', 'PROMPT_COMMAND': 'printf
"\\033]0;\\007"', 'PS1': ' \\s-\\v\\$ ', 'LANG':
'en_US.UTF-8'} and shell False
DEBUG: Unsharing. Flags: 1073741824
DEBUG: Unsharing. Flags: 134217728
Building target platforms: x86_64
Building for target x86_64
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.5b8t2s
+ umask 022
+ cd /builddir/build/BUILD
+ cd /builddir/build/BUILD
+ rm -rf warp-0.1.15
+ /usr/bin/gzip -dc /builddir/build/SOURCES/warp-0.1.15.crate
+ /usr/bin/tar -xof -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd warp-0.1.15
+ /usr/bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ set -eu
+ /usr/bin/mkdir -p .cargo
+ cat
+ /usr/bin/rm -f Cargo.lock
+ exit 0
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.4iJY1v
+ umask 022
+ cd /builddir/build/BUILD
+ cd warp-0.1.15
+ /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 /usr/bin/cargo build -j8 -Z
avoid-dev-deps --release
error: no matching package named `bytes` found
```
This means that `%generate_buildrequires` was not executed.
[rust-warp-0.1.15-1.fc31.src.rpm.gz](https://github.com/rpm-software-management/rpm/files/3061081/rust-warp-0.1.15-1.fc31.src.rpm.gz)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481450123___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
FYI: https://github.com/rpm-software-management/mock/commit/1c73a7ac7659d852a4d38e4bd5ba4641e224b206 Copr build with this commit is here https://copr.fedorainfracloud.org/coprs/g/mock/mock/package/mock/ I did not test it yet. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481309057___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ignatenkobrain can you please send me some spec/src.rpm with dynamic dependencies? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481295283___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
I have built 4.14 RPM with necessary changes (obviously, only for testing. f28/f29/f30/f31): https://copr.fedorainfracloud.org/coprs/ignatenkobrain/rpm-buildreqs/ -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-481248610___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
In Mock, I plan to have this workflow: 1) rpmbuild 2) if exit code == 11 2.1) dnf install $(rpm -qpR foo.buildreqs.nosrc.rpm) 2.2) rm foo.buildreqs.nosrc.rpm 3) rpmbuild But yeah, you can do the cleanup in rpmbuild as well. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480994790___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
The buildreqs.rpm is placed in the SRPMS/ dir right now (as it uses the srpm code path). But Panu is right. We need to clean it up, upon successful build. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480993075___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> Since the buildreqs package is a short-lived artifact and not something > people would want to hang on to, I think we should cleanup step on successful > build, or maybe use /var/tmp to create it in the first place. And might want > to have the directory configurable separate from src.rpm creation (eg to > create buildreq rpms into some tmp location but src.rpm's into the SRPMS for > keeping) Please no `/var/tmp`. It should be something which Mock cleans on every run and where it cannot be confused with some other files (consider if `foo.src.rpm` has `Name: bar`). I think `~/rpmbuild/SRPM/` would be nice. @ffesti your current code place it where? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480892823___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +if (Ferror(fd)) {
+ rpmlog(RPMLOG_ERR, _("Unable to open temp file: %s\n"), Fstrerror(fd));
+ goto exit;
+}
+
+if ((fp = fdopen(Fileno(fd), "w")) == NULL) {
+ rpmlog(RPMLOG_ERR, _("Unable to open stream: %s\n"), strerror(errno));
+ goto exit;
+}
+
+fprintf(fp, "cd '%s'\n", buildDir);
+
+if (spec->buildSubdir)
+ fprintf(fp, "cd '%s'\n", spec->buildSubdir);
+fprintf(fp, "%s", getStringBuf(spec->buildrequires));
+(void) fclose(fp);
Something like this: https://github.com/rpm-software-management/rpm/pull/659
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#discussion_r273062910___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
pmatilai commented on this pull request.
> +if (Ferror(fd)) {
+ rpmlog(RPMLOG_ERR, _("Unable to open temp file: %s\n"), Fstrerror(fd));
+ goto exit;
+}
+
+if ((fp = fdopen(Fileno(fd), "w")) == NULL) {
+ rpmlog(RPMLOG_ERR, _("Unable to open stream: %s\n"), strerror(errno));
+ goto exit;
+}
+
+fprintf(fp, "cd '%s'\n", buildDir);
+
+if (spec->buildSubdir)
+ fprintf(fp, "cd '%s'\n", spec->buildSubdir);
+fprintf(fp, "%s", getStringBuf(spec->buildrequires));
+(void) fclose(fp);
This script-running part with all the templating etc needs to be merged into
doScript() really.
Make doScript() always use rpmfcExec() and return its output via pointer if
supplied, and parse the output in the caller.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-223839526___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
The buildreqs rpm does indeed seem a bit weird, but probably makes a lot of sense in practise. nosrc.rpm is indeed better than src.rpm. Since the buildreqs package is a short-lived artifact and not something people would want to hang on to, I think we should cleanup step on successful build, or maybe use /var/tmp to create it in the first place. And might want to have the directory configurable separate from src.rpm creation (eg to create buildreq rpms into some tmp location but src.rpm's into the SRPMS for keeping) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480824529___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@Conan-Kudo but to retrieve `%generate_buildrequires` you need to execute `%prep` which is a non-trivial task. And somehow pass the output. I actually like the `.buildreqs` as parsing Requires is well defined and you can do that well as human and as a machine. @ffesti `.buildreqs.nosrc.rpm` sounds fine to me. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480816171___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
> @ffesti Is there a reason we can't just do this by evaluating the spec file > instead of producing weird (no)src rpms? Yes. If the dynamic dependencies go into a spec file there is the risk of the spec file getting built. That's not what we want. The dynamic dependencies need to stay dynamic. The spec file should not be changed. Also, if your parser is good enough you can just parse the %generate_buildrequires section and use the result of that... ;) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480813256___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)
@ffesti Is there a reason we can't just do this by evaluating the spec file instead of producing weird (no)src rpms? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/593#issuecomment-480808138___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
