Fix compilation ways #2286

Merged
merged 7 commits into from Dec 19, 2014

Projects

None yet

3 participants

@ttuegel
Member
ttuegel commented Dec 18, 2014

This fixes a number of bugs/odd corner cases with different compilation ways:

  • HPC can now generate reports even if ways besides the default are enabled.
  • --enable-executable-profiling is now --enable-profiling and implies --enable-library-profiling. Otherwise, the build will fail if the executable depends on the library, which is the usual case. You can still try to build the executables with profiling and the library without by passing --enable-profiling --disable-library-profiling to configure.
  • --enable-executable-dynamic now implies --enable-shared. Otherwise, the build will fail if the executable depends on the library, which is the usual case. You can still try to build the executables with dynamic linking without building a shared library by passing --enable-executable-dynamic --disable-shared to configure.
  • Test cases for all these issues are added.
  • The documentation for these options is updated (or added, in the case of the previously undocumented options).

Fixes #2281.

Thomas Tuegel added some commits Dec 13, 2014
Thomas Tuegel PackageTests: add full range of TestSuite/Hpc tests
Also runs the HPC tests regardless of the detected version.
6af70b3
Thomas Tuegel Build shared library when linking executables dynamically
Dynamically linking executables will fail without a shared library
unless the executables do not depend on the library or there is no
library. If there is no library, building shared libraries comes at no
cost. If there is a library, the executables usually depend on it, so it
makes sense to --enable-shared.

If the user passes --disable-shared, it will still be honored, but a
warning will be produced.
2976fef
Thomas Tuegel Enable library profiling when profiling executables
It doesn't make any sense to do a profiling build of an executable if
the library doesn't have a profiled build! The option
--enable-executable-profiling is changed to --enable-profiling to
reflect that it is now a global flag. The old option is still recognized
(for now).
6b63174
Thomas Tuegel PackageTests: set dist prefix
Some package tests run multiple tests on the same package, causing the
build directory to be overwritten. For debugging, it is important to
keep the build directory contents, so in this case we run each test
with a different build directory.
61f97e0
Thomas Tuegel hpc: separate module interfaces by compilation way 3974e36
Thomas Tuegel Document --enable-coverage 40b0df9
@23Skidoo
Member

LGTM.

@23Skidoo

LGTM.

@23Skidoo

LGTM.

@23Skidoo

LGTM.

@23Skidoo

Makes sense. We could of course detect if executables actually depend on the library, but that just adds complexity without clear benefit.

@ttuegel
Member
ttuegel commented Dec 18, 2014

Three Travis tests are failing due to HPC being too old. I may have to disable that test again. The GHC 7.8 test is failing because the test suite checks dynamic linking, and for some reason, the dynamically linked executables cannot find the library. I will have to investigate later.

@23Skidoo

We should refactor this whole fragment at some point, it's too complex (-dyn-too, phase reordering, and now this).

@23Skidoo

LGTM.

@23Skidoo
Member

LGTM.

The option --enable-executable-profiling is changed to --enable-profiling to
reflect that it is now a global flag. The old option is still recognized
(for now).

Does this mean we now have both enable-profiling and executable-profiling in the default ~/.cabal/config?

@ttuegel
Member
ttuegel commented Dec 19, 2014

Does this mean we now have both enable-profiling and executable-profiling in the default ~/.cabal/config?

I don't really understand how the config file parser works, but it looks like it generates field descriptions based on the command-line option parser? If that's the case, then profiling and executable-profiling are both recognized fields, but neither would appear in the default config, as the default for each is NoFlag.

@ttuegel
Member
ttuegel commented Dec 19, 2014

The GHC 7.8 test is failing because the test suite checks dynamic linking, and for some reason, the dynamically linked executables cannot find the library.

This has something to do with relocatable packages and our RPATH handling, see #2255.

@ttuegel
Member
ttuegel commented Dec 19, 2014

I have fixed up the HPC tests again so they don't run when HPC is too old to perform test coverage properly (before GHC 7.8). The remaining test failures are RPATH stuff, unrelated to my changes, so I'm going to merge despite the failures.

@ttuegel ttuegel merged commit b1ab958 into haskell:master Dec 19, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build failed
Details
@ttuegel ttuegel deleted the ttuegel:hpc branch Dec 19, 2014
@tibbe tibbe commented on the diff Dec 19, 2014
Cabal/Distribution/Simple/Setup.hs
@@ -450,8 +450,8 @@ configureOptions showOrParseArgs =
configDynExe (\v flags -> flags { configDynExe = v })
(boolOpt [] [])
- ,option "" ["executable-profiling"]
- "Executable profiling"
+ ,option "" ["profiling", "executable-profiling"]
@tibbe
tibbe Dec 19, 2014 Member

I don't 100% understand why we added another flag for the same thing.

@23Skidoo
23Skidoo Dec 19, 2014 Member

--profiling is the new name of --executable-profiling, which now also entails library-profiling. Old name is preserved for backwards compat.

@23Skidoo
Member

If that's the case, then profiling and executable-profiling are both recognized fields, but neither would appear in the default config, as the default for each is NoFlag.

Can you please check? We want only profiling to appear in the default config file.

@ttuegel
Member
ttuegel commented Dec 19, 2014

Can you please check? We want only profiling to appear in the default config file.

I checked, and only profiling appears in the default config, no executable-profiling.

@23Skidoo
Member

Nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment