Bug 199500 - [patch] lang/clang35 (all clang actually) : fails to set environment
Summary: [patch] lang/clang35 (all clang actually) : fails to set environment
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Brooks Davis
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-04-17 15:59 UTC by John Marino
Modified: 2015-05-19 14:20 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (brooks)


Attachments
patch to set environment during building targets (789 bytes, patch)
2015-04-17 16:00 UTC, John Marino
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Marino freebsd_committer freebsd_triage 2015-04-17 15:59:57 UTC
Each port needs to set the enviroment.  This is handled by the Mk framework, but when the build targets are customized, those targets need to handle it.  

The build problem on DragonFly with the new compiler was escaping me for a long time until I really that _ALL_ clang ports are failing to set the environment.  It was failing on unit tests.

The attached patch will fix this problem.

It also makes additional changes:
  * Changes two commands to one by using -C argument for the MAKE_CMD
  * unsuppresses the build commands  (build commands are always supposed to be shown)

It also sets the environment in the post-build target although this is less critical.

I did not set the environment in the post-install target that uses MAKE_CMD because it was just installing


FYI -- this isn't just pedantic, DragonFly toolchain relies on the environment to switch between the two base compilers and the two sets of binutils (and the two linkers).

A similar change needs to be made to all clang ports.  I can provide patches if necessary.
Comment 1 John Marino freebsd_committer freebsd_triage 2015-04-17 16:00:51 UTC
Created attachment 155677 [details]
patch to set environment during building targets

Oops, here is the patch.
Comment 2 Brooks Davis freebsd_committer freebsd_triage 2015-04-17 16:12:49 UTC
If is required there needs to be a single variables provided by the Mk system that does this.  IMO it should probably be spelled MAKE_CMD.
Comment 3 John Marino freebsd_committer freebsd_triage 2015-04-17 16:17:58 UTC
single variable?  you mean like "MAKE_ENV" ?

It is required -- see the build targets in bsd.port.mk, they all set the environment.
Comment 4 Brooks Davis freebsd_committer freebsd_triage 2015-04-19 19:29:34 UTC
If virtually all uses of MAKE_CMD are going to be "${SETENV} ${MAKE_ENV} ${MAKE_CMD}" then there should be a single variable that contains all of that whose use should be encouraged.  That's doubly true if failures will only occur on non-FreeBSD platforms where the average porter can't test and IMO can't be expected to care.
Comment 5 John Marino freebsd_committer freebsd_triage 2015-04-19 19:45:32 UTC
I can think of lots of places that suggestion doesn't work.  Most importantly, it is easier to fix 6 ports than sweep the entire tree.

Is there a problem with the patch provided?  Do you need patches for the clang ports?  I'm not understanding the reason for the push-back given that it's clear the environment wasn't set.
Comment 6 Brooks Davis freebsd_committer freebsd_triage 2015-04-20 15:07:12 UTC
If you give me a tested patch for all of them I will commit it.

I'm one of the mostly silent set of people who thinks DragonFly support isn't something they want to expend effort on.  Anything you can do to reduce the effort required will help your cause.
Comment 7 John Marino freebsd_committer freebsd_triage 2015-04-20 15:10:09 UTC
okay, but removing DragonFly from the equation doesn't suddenly make this mistake "correct".

If effort is the main problem, might I suggest you review the *tested* patch already attached, and if it passes give me approval to commit it and similar patches to each port.  Then you don't have to do anything besides review.
Comment 8 Brooks Davis freebsd_committer freebsd_triage 2015-04-20 15:12:24 UTC
The patch is acceptable.  Please commit to all the clang ports.
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-05-19 14:19:32 UTC
A commit references this bug:

Author: marino
Date: Tue May 19 14:18:33 UTC 2015
New revision: 386779
URL: https://svnweb.freebsd.org/changeset/ports/386779

Log:
  lang/clang3*, lang/clang-devel: set environment during build

  The clang ports override the default build target, but the new targets
  fail to set the environment like the default target does.  This patch
  passes MAKE_ENV to environment.

  It also combines compounds to a single make cmd and unsuppresses the
  commands so the output shows on the build logs.

  PR:		199500
  Submitted by:	marino
  Approved by:	brooks (maintainer)

Changes:
  head/lang/clang-devel/Makefile
  head/lang/clang33/Makefile
  head/lang/clang34/Makefile
  head/lang/clang35/Makefile
  head/lang/clang36/Makefile
Comment 10 John Marino freebsd_committer freebsd_triage 2015-05-19 14:20:46 UTC
Thanks!

I didn't forgot, but I was having trouble finding the time to run all the clang ports through poudriere.

I did that today.  All of these built fine on FreeBSD 10 /amd64 jail.