Bug 233546 - devel/psptoolchain-gcc-stage1: Fix build conflict when texinfo installed and when -march is set in env
Summary: devel/psptoolchain-gcc-stage1: Fix build conflict when texinfo installed and ...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Kurt Jaeger
Keywords: easy
Depends on:
Blocks: 238769
  Show dependency treegraph
Reported: 2018-11-26 21:21 UTC by Tassilo Philipp
Modified: 2019-06-30 20:25 UTC (History)
3 users (show)

See Also:
tphilipp: maintainer-feedback+
koobs: merge-quarterly?

patch (834 bytes, patch)
2018-12-05 22:11 UTC, Tassilo Philipp
tphilipp: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tassilo Philipp 2018-11-26 21:21:18 UTC
Port doesn't build when print/texinfo is installed. The post-patch target in the port's Makefile already disables info file generation (as unneeded for this build), but that doesn't seem to be enough.

Maybe this could all be made simpler with MAKEINFO=true for the build instead of the hacky patches in post-patch.
Comment 1 Tassilo Philipp 2018-11-26 21:26:49 UTC
just to avoid confusion, as the bug creation led to sunpoet@ (maintainer of print/texinfo) being automatically added to cc: this is not a problem with the texinfo port, and I opened this as the maintainer of the psptoolchain-* ports myself to have it written up somewhere, first
Comment 2 Tassilo Philipp 2018-12-05 22:11:07 UTC
Created attachment 199868 [details]

patch attached that fixes this; it also fixes another issue and guarantees that the built xgcc never receives any wrong -march flags from the environment, e.g. when CPUTYPE is set in make.conf
Comment 3 Tassilo Philipp 2018-12-14 14:23:34 UTC
Ping? Can we get this in?
Comment 4 Tassilo Philipp 2019-01-21 09:48:21 UTC
Ping again? The same changes in two other related ports (PRs #233812 and #233813) where pushed quite a while ago, so I'm not sure what this one is blocked by.
Comment 5 Tassilo Philipp 2019-02-25 15:04:08 UTC
Any updates on when this will make it in? Thanks
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-03 02:13:00 UTC
Confirmation that this change passes QA (portlint, poudriere) would be helpful
Comment 7 Tassilo Philipp 2019-06-04 11:52:21 UTC
I am afraid I don't fully understand the process, so please bear with me. Two identical fixes to related ports have made it in months ago, however this one didn't and was tagged "needs-qa". In all cases I am the maintainer of the ports, the reporter of the issue and the one who submitted the patches. As maintainer I also gave feedback and patch approval.
What exactly is missing here?

I can provide you a log of portlint and poudriere build, if you want that. I just have the feeling that I don't understand something in the process, so could you please explain to me what I'm missing?
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-04 11:56:41 UTC
(In reply to Tassilo Philipp from comment #7)

@Tassilo You're not missing anything, but perhaps I could have been clearer. 

Confirmation that the change passes QA (logs are not required), can often help get an issue progressed faster, particularly if/when committers are not familiar with the port.

Once confirmation is provided, the needs-qa is no longer required and we can remove it.

Other than that there's nothing I can see preventing this from being committed, except someone to assign themselves and do so

Apologies for the ambiguity
Comment 9 Tassilo Philipp 2019-06-04 12:12:11 UTC
Ok, so just to clear up my confusion: for who is the "needs-qa" flag? Is it something the submitter of the patch, the maintainer, or the comitter needs to look at?

If it's the maintainer, does it mean that in this case, when you originally set "needs-qa" in december, that all I needed to do was to say "confirmed that it works" (as the maintainer)?

If so, then I apologize for not having done so, I really interpreted this to be something internal to the ports team. The reason I removed it a few days ago was that I was getting a bit frustrated with this, as I didn't know what to do and didn't have any answers.
Comment 10 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-04 12:24:26 UTC
(In reply to Tassilo Philipp from comment #9)

It's for anyone looking at the issue (to ascertain its 'current state', but it mostly applies to committers looking to take/assign the issue to themselves, and maintainers.

I should have @Tassilo when i said "Confirmation that this change passes QA (portlint, poudriere) would be helpful", but i did also set maintainer-feedback to ? your-email

In future you can take it when someone sets that flag to 'we would like something from you', either a question answered, additional information, an updated patch, or in this case, QA confirmation

As far as what kind of QA is desired, in most if not all cases, portlint and poudriere passing is the best kind (at minimum), not including runtime tests or test suites passing, which of course are better.
Comment 11 Tassilo Philipp 2019-06-05 14:27:55 UTC
Thanks for the explanations, I think it's clear now.

So to confirm, this passes poudriere and also portlint.

That said, the patch attached in december applies with a fuzz of one line now, b/c this here was applied last month: https://reviews.freebsd.org/rP499897#change-iee5fTGYlh3m

Ignoring a potential problem I see with that change that introduced the fuzz (see #238249), I think this is good to go. I confirm the patch still applies correctly with that fuzz.

Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-05 21:26:25 UTC
Thanks Tassilo!
Comment 13 Tassilo Philipp 2019-06-07 13:32:40 UTC
@Kubilay I have a question: I have an upcoming upgrade of this port to a newer version. Since this patch here isn't committed yet, I'm thinking about merging the patch into the bigger one that will be used for the upgrade, then close this PR here. Does that sound like an ok approach, or would it be better to wait for this PR here to be committed?
I don't want this here to block the upgrade indefinitely, it's already uncomitted for 6+ months...
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-11 05:11:19 UTC
(In reply to Tassilo Philipp from comment #13)

Bug fixes (upstream or ports bugs), should be separated from version/feature updates, so that those bug fixes can be merged to the quarterly branch, unlike feature/version updates, which cannot. 

Bundling them together means bugfixes don't make their way to quarterly branch users
Comment 15 Tassilo Philipp 2019-06-11 16:37:32 UTC
(In reply to Kubilay Kocak from comment #14)

Thanks for the info. Makes sense.

Unfortunately this blocks the mentioned update, for now, which is also needed as it would implicitly fix another issue (#238249). Oh well, I hope that this here will get committed, soon.
Comment 16 Kubilay Kocak freebsd_committer freebsd_triage 2019-06-15 04:24:22 UTC
(In reply to Tassilo Philipp from comment #15)

My pleasure Tassilo

Create a newissue for the update, put this bugs issue id in that new issues "depends on" field.

Add bug 238249 to the new issues "blocks" field
Comment 17 Kurt Jaeger freebsd_committer 2019-06-30 20:24:54 UTC
Committed, thanks!
Comment 18 commit-hook freebsd_committer 2019-06-30 20:25:08 UTC
A commit references this bug:

Author: pi
Date: Sun Jun 30 20:24:40 UTC 2019
New revision: 505480
URL: https://svnweb.freebsd.org/changeset/ports/505480

  devel/psptoolchain-gcc-stage1: Fix build conflict

  - Fix build conflict when texinfo installed and when -march is set in env

  PR:		233546
  Submitted by:	Tassilo Philipp <tphilipp@potion-studios.com> (maintainer>
  Reviewed by:	koobs