Opened 4 years ago

Closed 18 months ago

Last modified 11 months ago

#10536 closed task (fixed)

Clear up how to turn off dynamic linking in build.mk

Reported by: thomie Owned by: alpmestan
Priority: normal Milestone: 8.6.1
Component: Build System (make) Version: 7.10.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D1021
Wiki Page:

Change History (12)

comment:1 Changed 4 years ago by Thomas Miedema <thomasmiedema@…>

In 37de4ad76b75d403e6a8dae9539af08c859d46a4/ghc:

Build system: don't set GhcLibWays explicitly in build.mk.sample (#10536)

We used to have the following in mk/build.mk.sample:

  GhcLibWays = $(if $(filter $(DYNAMIC_GHC_PROGRAMS),YES),v dyn,v)

This commit removes that statement for the following reasons:

  1) It depends on the variable DYNAMIC_GHC_PROGRAMS, which is set later
     in the file for some BuildFlavours. Although this works because
     `make` does multiple passes when reading Makefiles, it is confusing
     to users [1]. Instead, test for DYNAMIC_GHC_PROGRAMS in
     mk/config.mk.in.

  2) Although it looks like that line is about compiling the `dyn` way,
     its purpose is really to not build the `prof` way. This commit
     introduces the variable BUILD_PROF_LIBS, to make this more
     explicit.

This simplifies mk/build.mk.sample and mk/validate-settings.mk.

Note that setting GhcLibWays explicitly still works, and
DYNAMIC_GHC_PROGRAMS=NO in build.mk does not build the `dyn` way.

[1] https://mail.haskell.org/pipermail/ghc-devs/2014-December/007725.html

Differential Revision: https://phabricator.haskell.org/D1021

comment:2 Changed 4 years ago by thomie

Differential Rev(s): Phab:D1021

bgamari mentioned in Phab:D1021:

The exact semantics of DYNAMIC_GHC_PROGRAMS actually perplexed me quite a bit at first, in large part due to this overloading of roles. Perhaps it would make sense to just tear off the band-aid: We could retain DYNAMIC_GHC_PROGRAMS and its associated logic for the time being, emitting a warning if we find it set. Alongside it we could introduce a new DYNAMIC_GHC_EXECUTABLES (or some other name) flag strictly intended to control whether we link ghc and friends dynamically. Finally, we could introduce another flag for specifying whether dynamic libraries should be built. Eventually DYNAMIC_GHC_PROGRAMS could be dropped.

thoughtpolice:

I'm somewhat against putting in all this work because it does sound confusing, but also because I think the state of shared library support is in the air (personally I'm coming around to the "Nuke it from orbit" position.) So this improvement is at least a cleanup with no substantial semantics change for right now.

Leaving this ticket open till we make up our mind about this.

comment:3 Changed 4 years ago by int-e

Could we still mention GhcLibWays in build.mk.sample? For example, under Other settings...:

# Set library ways directly instead of computing it from BUILD_PROF_LIBS and
# DYNAMIC_GHC_PROGRAMS (may result in build failures)
#GhcLibWays = v dyn p

On the topic of DYNAMIC_GHC_PROGRAMS, I believe the logic for enabling the dyn way is as follows: Since the flag implies that ghci is linked dynamically, using the dynamic rts, that means ghci requires the dyn libraries in order to work.

Also regarding the semantics, DYNAMIC_BY_DEFAULT is another option which affects the usability of the resulting ghc: on Linux (presumably the story on Windows is different) Cabal doesn't interact with the resulting ghc properly if the vanilla way is enabled in Cabal's configuration, resulting in obscure build failures.

comment:4 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:5 Changed 3 years ago by thoughtpolice

Milestone: 8.0.18.2.1

Bumping out to 8.2.x

comment:6 Changed 2 years ago by bgamari

Milestone: 8.2.18.4.1

Given that 8.2.1-rc1 is imminent, I'm bumping these off to the 8.4

comment:7 Changed 20 months ago by bgamari

Milestone: 8.4.18.6.1

This ticket won't be resolved in 8.4; remilestoning for 8.6. Do holler if you are affected by this or would otherwise like to work on it.

comment:8 Changed 18 months ago by bgamari

Owner: set to alpmestan

Assigning to Alp to ensure this happens in Hadrian.

comment:9 Changed 18 months ago by alpmestan

This is very easy to achieve with Hadrian and I just submitted a pull request against hadrian master that documents how it can be done. Will post another comment when it gets merged.

comment:10 Changed 18 months ago by alpmestan

The PR has been merged, this is now covered explicitly in hadrian's documentation.

comment:11 Changed 18 months ago by bgamari

Resolution: fixed
Status: newclosed

Lovely, let's close this then.

comment:12 Changed 11 months ago by bgamari

Component: Build SystemBuild System (make)

The new Hadrian build system has been merged. Relabeling the tickets concerning the legacy make build system to prevent confusion.

Note: See TracTickets for help on using tickets.