Bug 163226 - [patch] vietnamese/libviet: respect CC/CFLAGS
Summary: [patch] vietnamese/libviet: respect CC/CFLAGS
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: David E. O'Brien
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-13 06:10 UTC by Jan Beich
Modified: 2014-08-13 02:01 UTC (History)
1 user (show)

See Also:


Attachments
cc.diff (593 bytes, patch)
2011-12-13 06:10 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2011-12-13 06:10:28 UTC
Responsible Changed
From-To: freebsd-ports-bugs->obrien

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 David E. O'Brien freebsd_committer freebsd_triage 2012-09-29 00:39:33 UTC
State Changed
From-To: open->closed

This seems fixed to me. 

http://www.freebsd.org/cgi/query-pr.cgi?pr=163226 

Date: Fri, 28 Sep 2012 18:41:56 -0800
Comment 3 Beat Gaetzi freebsd_committer freebsd_triage 2012-10-20 13:13:09 UTC
State Changed
From-To: closed->open

Re-opened on submitters request.
Comment 4 John Marino freebsd_committer freebsd_triage 2014-07-28 07:45:04 UTC
libviet is building cleanly everywhere:
http://portsmon.freebsd.org/portoverview.py?category=vietnamese&portname=libviet
Comment 5 Jan Beich freebsd_committer freebsd_triage 2014-07-28 09:39:02 UTC
Opening again per #reply1 in (lost due to bug 190975)
https://www.freebsd.org/cgi/query-pr.cgi?pr=ports/163226
Comment 6 John Marino freebsd_committer freebsd_triage 2014-07-28 09:55:03 UTC
forget gnats - Can you explain what the current problem is again?

It is building on all releases as confirmed by portsmon.  Referring to a long-gone pointyhat log doesn't help me much.  All see is reference to failure to build with make CC=gcc46 but I don't see that as a requirement on the port (ports aren't properly set up to build with ports compiler -- i've submitted a disapproved patch to do this)
Comment 7 Jan Beich freebsd_committer freebsd_triage 2014-07-28 12:52:09 UTC
(In reply to John Marino from comment #6)
> forget gnats - Can you explain what the current problem is again?

The Porter's Handbook uses the verb "must" in regards to CC/CFLAGS. That's enough to consider any port that doesn't respect them as broken or not up to the expected standard of quality.

https://www.freebsd.org/doc/en/books/porters-handbook/dads-cc.html
https://www.freebsd.org/doc/en/books/porters-handbook/dads-cflags.html

Using Environment field in GNATS it's easy to construct the steps to reproduce:

  $ export PATH=~/.bin:$PATH
  $ for cc in CC cc c++ gcc g++ cpp; do ln -s /usr/bin/false ~/.bin/${cc}; done
  $ cd vietnamese/libviet
  $ echo CC=clang >${__MAKE_CONF-/etc/make.conf}
  $ make
  ===>  Building for vi-libviet-20010210_1
  CFLAGS are -O2 -pipe  -fno-strict-aliasing
  Working on viqr         {
  cc  -I../../include -c vncompos.c
  *** Error code 1

Notice how the advertised CFLAGS aren't actually used to build vncompos.c.

> It is building on all releases as confirmed by portsmon.

Per bug 159117 comment 4 portmgr@ stopped doing -exp runs with non-default CC and /usr/bin/cc removed. After that /stable/9 was used as the base for -exp runs and actually tested whether "gcc as cc" can build most ports, embracing false positives until the switch to "clang as cc".

> All see is reference to
> failure to build with make CC=gcc46 but I don't see that as a requirement on
> the port (ports aren't properly set up to build with ports compiler -- i've
> submitted a disapproved patch to do this)

portmgr@ should forbid/strongly discourage local builds with non-default environment first before declaring such bugs as "Works As Intended" or "Not A Bug". Documentation needs to be adjusted as well. Here's an example for Mk/bsd.port.mk:

  .if !defined(PACKAGE_BUILDING)
  IGNORE= Due to prevalence of hard to debug build errors local builds are no longer supported. Any bug filed for such an environment even with patches attached won't be looked at by a committer.
  .endif
Comment 8 John Marino freebsd_committer freebsd_triage 2014-07-28 13:06:08 UTC
(In reply to Jan Beich from comment #7)
> (In reply to John Marino from comment #6)
> > forget gnats - Can you explain what the current problem is again?
> 
> The Porter's Handbook uses the verb "must" in regards to CC/CFLAGS. That's
> enough to consider any port that doesn't respect them as broken or not up to
> the expected standard of quality.
> 
> https://www.freebsd.org/doc/en/books/porters-handbook/dads-cc.html
> https://www.freebsd.org/doc/en/books/porters-handbook/dads-cflags.html


That says that *any* port violating either this CFLAGS or CC edict must be designated "NO_PACKAGE".  Thankfully that rule is globally ignored or there would be riot by the users.  I have no idea how many of the 25,000 ports violate either of these rules, but I'd hazard a guess that it's "a lot"

Recently we've had something like 2200 PRs just on PRs.  Through aggressive processing, we've gotten it down to around 1700 or so.  We've got real problem, especially with staging and a huge backlog of new ports.  

Given that, why look to promote trivial issues?  In the scheme of things, this is not a problem.  The builders can package it, so this seems academic.



> Per bug 159117 comment 4 portmgr@ stopped doing -exp runs with non-default
> CC and /usr/bin/cc removed. After that /stable/9 was used as the base for
> -exp runs and actually tested whether "gcc as cc" can build most ports,
> embracing false positives until the switch to "clang as cc".


So maybe we bring this up again once ignoring CFLAGS becomes the top problem?

> portmgr@ should forbid/strongly discourage local builds with non-default
> environment first before declaring such bugs as "Works As Intended" or "Not
> A Bug". Documentation needs to be adjusted as well. Here's an example for
> Mk/bsd.port.mk:
> 
>   .if !defined(PACKAGE_BUILDING)
>   IGNORE= Due to prevalence of hard to debug build errors local builds are
> no longer supported. Any bug filed for such an environment even with patches
> attached won't be looked at by a committer.
>   .endif

I'm confused here.  Are you saying because of an abundance of errors such as libviet has across the tree, portmgr says not to consider them an error?  If so, isn't that what both obrien and I just did?
Comment 9 John Marino freebsd_committer freebsd_triage 2014-07-28 13:29:36 UTC
So I see now that the CC/CFLAGS thing was the entire point of this PR.

Why not just provide a patch that fixes it?  Since this is clearly a "maintainer timeout" it can be committed immediately.  While I am not particularly inclined to come up with a patch myself and obviously obrien isn't either, I will happily commit a patch for you.
Comment 10 Jan Beich freebsd_committer freebsd_triage 2014-07-28 15:13:21 UTC
(In reply to John Marino from comment #8)
> Given that, why look to promote trivial issues?  In the scheme of things,
> this is not a problem.  The builders can package it, so this seems academic.

How trivial a problem this is depends on how much you understand the implications. Not respecting CC blocks a port from using experimental compiler features on it not available in base clang or gcc e.g. ASan, LTO, Polly/Graphite or cross-arch build. Not respecting CFLAGS blocks a port from supporting DEBUG symbols as well as userland profiling with dtrace or pmcstat.

(In reply to John Marino from comment #9)
> Why not just provide a patch that fixes it?

Why not look at the PR thoroughly? It already contains attachment 120422 [details] that provides the fix and still applies fine.
Comment 11 commit-hook freebsd_committer freebsd_triage 2014-07-28 15:28:48 UTC
A commit references this bug:

Author: marino
Date: Mon Jul 28 15:28:15 UTC 2014
New revision: 363179
URL: http://svnweb.freebsd.org/changeset/ports/363179

Log:
  vietnamese/libviet: respect CC/CFLAGS

  PR:		163226
  Submitted by:	Jan Beich
  Approved by:	maintainer timeout (~3 years)

Changes:
  head/vietnamese/libviet/Makefile
Comment 12 John Marino freebsd_committer freebsd_triage 2014-07-28 15:36:59 UTC
(In reply to Jan Beich from comment #10)
> How trivial a problem this is depends on how much you understand the
> implications. Not respecting CC blocks a port from using experimental
> compiler features on it not available in base clang or gcc e.g. ASan, LTO,
> Polly/Graphite or cross-arch build. Not respecting CFLAGS blocks a port from
> supporting DEBUG symbols as well as userland profiling with dtrace or
> pmcstat.

The "features of non-base compiler" argument doesn't sway me that much, I'd consider that "nice to have".  The lack of debugging is a bit more persuasive but I'd counter that you don't have WITH_DEBUG on indiscriminately(I ran with this in poudriere for a bit and it's not practical over the entire tree although it could be done for a selected subset of the tree).  So ultimately it's a "yeah, ideally CC/CFLAGS should be respected and make that happen if working on the port for another reason" but I'm not necessarily driven to fix it as the only problem.  I am just speaking for myself of course.

Anyway, your fix is in now so so the discussion is really academic at this point.


> Why not look at the PR thoroughly? It already contains attachment 120422 [details]
> [details] that provides the fix and still applies fine.

Barely (max fuzz). :) 
I didn't see it before.
Comment 13 Jan Beich freebsd_committer freebsd_triage 2014-07-28 16:44:18 UTC
>> Why not look at the PR thoroughly? It already contains attachment 120422 [details]
>> [details] that provides the fix and still applies fine.
>
> Barely (max fuzz). :) 
> I didn't see it before.

max-fuzz is 2 by default. |svn patch| doesn't reject a patch with fuzz unlike, say, |git am|.

> @@ -16,6 +16,8 @@
>  
>  OPTIONS_DEFINE=	DOCS
>  
> +#MAKE_ARGS=	${MAKE_ENV:S/CFLAGS/FLAGS/}
> +

How did this end up being commented out? It's not FIXED then.
Comment 14 John Marino freebsd_committer freebsd_triage 2014-07-28 18:05:11 UTC
I don't know.  "svn patch" makes a lot of errors, particular with leaving off line endings and combining lines.  It looks like it screwed up again.
Comment 15 John Marino freebsd_committer freebsd_triage 2014-07-28 18:06:22 UTC
ah, no, I remember.
I was testing it.  I forgot to remove the comment.

This time svn patch is not the culprit, I was.
Comment 16 commit-hook freebsd_committer freebsd_triage 2014-07-28 18:08:58 UTC
A commit references this bug:

Author: marino
Date: Mon Jul 28 18:08:48 UTC 2014
New revision: 363211
URL: http://svnweb.freebsd.org/changeset/ports/363211

Log:
  vietnamese/libviet: Fix last commit

  I was testing change and forgot to remove the hash before committing.

  PR:		163226
  reported by:	Jan Beich

Changes:
  head/vietnamese/libviet/Makefile