DavidEichmann (David Eichmann)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 1 2018, 9:55 AM (19 w, 6 d)

Recent Activity

Today

DavidEichmann added a comment to D4953: Support for deprecating exports.

Hello, sorry for the delay here. I can't seem to build this patch and am seeing merge conflicts. Can you please rebase on master, and resolve any merge conflicts.

Tue, Dec 18, 9:13 AM

Yesterday

DavidEichmann added a comment to D5409: Hadrian: support extra libraries + OSX rpath.

Keep in mind that the dynamic ghc build has been disabled in D5430 due to the failing hadrian build.

Mon, Dec 17, 3:32 PM

Tue, Dec 11

DavidEichmann added a comment to D5430: Revert dynamically linking ghc..

The build failure in question is failing to find libffi when building the default flavour on x86_64 with no system wide libffi installed. The command that fails is when linking the ghc binary. The make build system links the ghc binary with -lffi. On that grounds I would say that D5427 will not solve this issue that this patch does.

Tue, Dec 11, 9:16 AM
DavidEichmann added a comment to D5427: hadrian: Only link against libffi when it is needed for adjustors.

There seems to be some logic using -l$$(LIBFFI_NAME) in rts/ghc.mk. configure.ac also seems to set the default value of the ffi flag by doing AC_SUBST([CabalHaveLibffi],[False]) or AC_SUBST([CabalHaveLibffi],[True]).

Tue, Dec 11, 6:00 AM
DavidEichmann added a comment to D5427: hadrian: Only link against libffi when it is needed for adjustors.

Im confused: setting the ffi flag *enables* linking with ffi. So if I understand correctly, this can only serve to *increase* the cases where we link with ffi. I'm trying to find the relevant logic in the make build system now.

Tue, Dec 11, 5:19 AM
DavidEichmann added a comment to D5423: Fix hadrian default build cannot find -lffi..

Funny enough, I just came accross this in the make build system rts/ghc.mk:127:

Tue, Dec 11, 5:06 AM

Mon, Dec 10

DavidEichmann added a comment to D5430: Revert dynamically linking ghc..

I've tested locally using a nix environment where the Hadrian default build fails on master. With this patch I successfully built and ran tests for flavours: default, perf, quick, and quickest.

Mon, Dec 10, 11:20 AM
DavidEichmann planned changes to D5423: Fix hadrian default build cannot find -lffi..
Mon, Dec 10, 11:01 AM
DavidEichmann added a comment to D5423: Fix hadrian default build cannot find -lffi..

After speaking with Alp, we've decided it is safer to simply revert the dynamic linking changes: D5430. This should fix the Hadrian build while a hack free solution can be found.

Mon, Dec 10, 11:00 AM
DavidEichmann added a reviewer for D5430: Revert dynamically linking ghc.: alpmestan.
Mon, Dec 10, 10:57 AM
DavidEichmann created D5430: Revert dynamically linking ghc..
Mon, Dec 10, 10:55 AM

Sun, Dec 9

DavidEichmann updated the diff for D5368: Do not save performance test results if worktree is dirty..

Rebase

Sun, Dec 9, 8:44 AM
DavidEichmann added a comment to D5423: Fix hadrian default build cannot find -lffi..

You're absolutely right @bgamari, this situation is not ideal, but I do fully intend to continue work on this. The Hadrian build is broken and the correct solution seems non-trivial, hence I saw this hacky solution as somewhat justified.

Sun, Dec 9, 7:46 AM

Fri, Dec 7

DavidEichmann added a comment to D5423: Fix hadrian default build cannot find -lffi..

This fixes the build on CircleCI: https://gitlab.staging.haskell.org/DavidEichmann/ghc/-/jobs/904. Using Alp's nix environment I was able to recreate the build failure by building the default flavour. In the same environment with this patch build succeeds and I have run the tests in the perf and default flavours. I still see the drop in test failures the we expect from dynamically linking ghc.

Fri, Dec 7, 5:27 PM
DavidEichmann added a reviewer for D5423: Fix hadrian default build cannot find -lffi.: alpmestan.
Fri, Dec 7, 5:20 PM
DavidEichmann created D5423: Fix hadrian default build cannot find -lffi..
Fri, Dec 7, 5:17 PM

Tue, Dec 4

DavidEichmann added inline comments to D5409: Hadrian: support extra libraries + OSX rpath.
Tue, Dec 4, 7:42 AM

Sun, Dec 2

DavidEichmann added a comment to rGHC79d5427e1f9d: Hadrian: support dynamically linking ghc.

There is a similar issue on linux where we link the system ffi instead of the local one. This fails e.g. on CircleCI as ld cannot find the ffi library. I don't know if this is related to the OSX failure, but I'll continue investigating this on linux.

Sun, Dec 2, 9:43 AM

Fri, Nov 30

DavidEichmann updated the diff for D5368: Do not save performance test results if worktree is dirty..

Included too many changes in the last diff.

Fri, Nov 30, 11:25 AM
DavidEichmann updated the diff for D5368: Do not save performance test results if worktree is dirty..

Converting check_output() result to bool is confusing and in this case is wrong. Comparing to byte string literal instead.

Fri, Nov 30, 11:20 AM
DavidEichmann retitled D5368: Do not save performance test results if worktree is dirty. from [Merge After D5367] Do not save performance test results if worktree is dirty. to Do not save performance test results if worktree is dirty..
Fri, Nov 30, 10:57 AM
DavidEichmann committed rGHC6e24a0bee0d0: Skip all performance tests if not in a git repo. (authored by DavidEichmann).
Skip all performance tests if not in a git repo.
Fri, Nov 30, 10:55 AM
DavidEichmann closed D5367: Skip all performance tests if not in a git repo..
Fri, Nov 30, 10:55 AM

Thu, Nov 29

DavidEichmann added a comment to D4953: Support for deprecating exports.

Hello, thanks for fixing that, great work! I see you also added a test which is always appreciated. Before I accept:

  • I still think throwing an exception is more valid. I.e. replace loadSrcInterface_maybe with loadSrcInterface (this change would also make the code a bit simpler). If you're not sure why, do ask. If you're convinced otherwise, I'm not gonna fight you on it.
  • The changes in .gitmodules should be reverted (as a rule of thumb, I'll only accept if I think it's ready to merge as is).
    • You can easily fix this without breaking your build by, first pushing your changes git push, then re-cloning ghc using git clone git://git.haskell.org/ghc --recurse-submodules (if you need help with this feel free to contact me on IRC at freenode #haskell my handle is DavidEichmann).
Thu, Nov 29, 11:01 AM
DavidEichmann abandoned D5329: Bump Cabal to 2.5.0.0 and fix build errors..

Cabal was already bumped in D5385.

Thu, Nov 29, 10:23 AM
DavidEichmann retitled D5281: Hadrian: support dynamically linking ghc from [Merge after D5385] Hadrian: support dynamically linking ghc to Hadrian: support dynamically linking ghc.
Thu, Nov 29, 10:20 AM
DavidEichmann retitled D5281: Hadrian: support dynamically linking ghc from Dynamically linked ghc uses relative path to shared objects. to [Merge after D5385] Hadrian: support dynamically linking ghc.
Thu, Nov 29, 6:47 AM
DavidEichmann updated the diff for D5281: Hadrian: support dynamically linking ghc.

Build ghc with -fPIC -dynamic if RTS has a dynamic way.
Don't link executables with -shared -dynload deploy.
These changes are all in hadrian/src/Settings/Builders/Ghc.hs

Thu, Nov 29, 6:23 AM

Wed, Nov 28

DavidEichmann retitled D5368: Do not save performance test results if worktree is dirty. from Do not save performance test results if worktree is dirty. to [Merge After D5367] Do not save performance test results if worktree is dirty..
Wed, Nov 28, 7:34 AM

Tue, Nov 27

DavidEichmann updated the diff for D5367: Skip all performance tests if not in a git repo..
  • Use git status instead of rev-parse
Tue, Nov 27, 6:56 AM
DavidEichmann accepted D5385: Hadrian: bump Cabal submodule, install extra dynamic flavours of libHSrts.
Tue, Nov 27, 6:27 AM
DavidEichmann updated the diff for D5367: Skip all performance tests if not in a git repo..
  • Fix logic to call Perf.append_perf_stat() only if canGitStatus.
  • Improve error message.
Tue, Nov 27, 5:28 AM

Mon, Nov 26

DavidEichmann added a comment to D5367: Skip all performance tests if not in a git repo..

@osa1 We use git notes to get the baseline performance numbers and to save the new numbers. Here we only skip performance tests if we both cannot get a baseline (and hence can't tell if tests pass or fail) nor have a way to save the results.

Mon, Nov 26, 10:10 AM
DavidEichmann updated the diff for D5367: Skip all performance tests if not in a git repo..
  • Replace is_git_repo with can_git_status.
Mon, Nov 26, 8:50 AM

Fri, Nov 23

DavidEichmann created D5374: Fix unused import warning.
Fri, Nov 23, 5:09 PM
DavidEichmann added inline comments to D5281: Hadrian: support dynamically linking ghc.
Fri, Nov 23, 3:14 AM

Thu, Nov 22

DavidEichmann added a comment to D5312: Fix unused-import warnings.

Validate now passes locally for me.

Thu, Nov 22, 7:05 AM
DavidEichmann updated the diff for D5312: Fix unused-import warnings.
  • Fix T4437 by removing DerivingVia from expectedGhcOnlyExtensions. Update comment in GHC.Magic.
Thu, Nov 22, 5:57 AM

Wed, Nov 21

DavidEichmann updated the diff for D5312: Fix unused-import warnings.
  • Bump Cabal, bytestring, containers, and text.
  • Fix unused matches error.
  • Remove unused imprt after rebase.
  • Fix GHC.Magic validation errors.
Wed, Nov 21, 12:51 PM
DavidEichmann updated the Trac tickets for D5368: Do not save performance test results if worktree is dirty..
Wed, Nov 21, 6:48 AM
DavidEichmann created D5368: Do not save performance test results if worktree is dirty..
Wed, Nov 21, 6:48 AM
DavidEichmann retitled D5367: Skip all performance tests if not in a git repo. from Skip all perormance tests if not in a git repo. to Skip all performance tests if not in a git repo..
Wed, Nov 21, 5:21 AM
DavidEichmann updated the diff for D5367: Skip all performance tests if not in a git repo..
  • Correct assignment of hasMetricsFile.
Wed, Nov 21, 5:11 AM
DavidEichmann created D5367: Skip all performance tests if not in a git repo..
Wed, Nov 21, 4:39 AM

Tue, Nov 20

DavidEichmann added a comment to D5312: Fix unused-import warnings.

There seems to be some errors when I validate locally. I'm working on this now.

Tue, Nov 20, 5:10 AM

Mon, Nov 19

DavidEichmann updated the diff for D5312: Fix unused-import warnings.
  • Fix unused matches error.
Mon, Nov 19, 3:03 PM
DavidEichmann updated the diff for D5312: Fix unused-import warnings.
  • Bump Cabal, bytestring, containers, and text.
Mon, Nov 19, 1:56 PM

Nov 13 2018

DavidEichmann added a comment to D4953: Support for deprecating exports.

Hi @alanasp, Have you managed to have a look at the issue since we last spoke? It would be great to see this to completion. I appreciate that a significant amount of effort has already gone into this patch. If you're short on time/resources, please let me know and I will investigate this further.

Nov 13 2018, 11:29 AM

Nov 12 2018

DavidEichmann added a comment to D5329: Bump Cabal to 2.5.0.0 and fix build errors..

We'll likely need to bump Cabal again soon, but this at least gets ghc to build with Cabal 2.5.0.0.

Nov 12 2018, 1:08 PM
DavidEichmann created D5329: Bump Cabal to 2.5.0.0 and fix build errors..
Nov 12 2018, 12:27 PM
DavidEichmann updated the diff for D5327: hadrian: build ghc-iserv-dyn in addition to ghc-iserv and ghc-iserv-prof, as it is required for 10+ tests.

Fix comment.

Nov 12 2018, 5:44 AM
DavidEichmann created D5327: hadrian: build ghc-iserv-dyn in addition to ghc-iserv and ghc-iserv-prof, as it is required for 10+ tests.
Nov 12 2018, 4:57 AM

Nov 8 2018

DavidEichmann updated the diff for D5312: Fix unused-import warnings.

More commit message fiddling (I'm not sure how arcanist handles this).

Nov 8 2018, 12:22 PM
DavidEichmann updated the diff for D5312: Fix unused-import warnings.

Update commit message explaining why ieLWrappedName change was reverted.

Nov 8 2018, 12:16 PM
Herald added a reviewer for D5312: Fix unused-import warnings: goldfire.
Nov 8 2018, 12:04 PM

Nov 5 2018

DavidEichmann added a comment to D4953: Support for deprecating exports.

@DavidEichmann The case that you pointed out indeed works as intended. I've made sure that if there is a non-deprecating way a symbol can be brought into context then that way will be used.

Nov 5 2018, 6:22 AM

Oct 31 2018

DavidEichmann accepted D5284: Fix for Trac #15611: Scope errors lie about what modules are imported..

Looks good to me.

Oct 31 2018, 5:11 AM
DavidEichmann added a reviewer for D5284: Fix for Trac #15611: Scope errors lie about what modules are imported.: DavidEichmann.
Oct 31 2018, 5:05 AM

Oct 30 2018

DavidEichmann added a comment to D4953: Support for deprecating exports.

@DavidEichmann Ok, it's actually self-export rather than self import. The example is here /libraries/base/GHC/Num.hs

@alanasp Ah, thank you! That makes a lot more sense now.

Oct 30 2018, 1:28 PM
DavidEichmann added a comment to D4953: Support for deprecating exports.

@alanasp I wasn't aware that ghc supported self imports. The concept sounds a bit absurd to me (why would a module ever import itself?). I know GHC 8.4.3 would complain about circular imports, which is what I expect. I would expect a similar error to be raised by the code in this patch, but perhaps I'm missing something. Can you give a concrete example? How did you run into this use case?

Oct 30 2018, 4:30 AM

Oct 29 2018

DavidEichmann added a reviewer for D5281: Hadrian: support dynamically linking ghc: alpmestan.
Oct 29 2018, 11:22 AM
DavidEichmann updated the diff for D5281: Hadrian: support dynamically linking ghc.

Minor cleanup.

Oct 29 2018, 11:21 AM
DavidEichmann created D5281: Hadrian: support dynamically linking ghc.
Oct 29 2018, 10:45 AM
DavidEichmann added a comment to D4953: Support for deprecating exports.

Regarding loadSrcInterface vs loadSrcInterface_maybe , it is hard to see what the correct solution is here. What exactly do you mean by "self import"? something like module A where; import module A?

Oct 29 2018, 3:53 AM

Oct 24 2018

DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..

Added CircleCI push git notes script. Removed temporary changes.

Oct 24 2018, 10:50 AM

Oct 17 2018

DavidEichmann added a comment to D5059: testsuite: Save performance metrics in git notes..

Please note there are still temporary changes that need to be removed from this patch.

Oct 17 2018, 3:45 AM

Sep 25 2018

DavidEichmann added inline comments to D4953: Support for deprecating exports.
Sep 25 2018, 11:35 AM

Sep 24 2018

DavidEichmann added inline comments to D4953: Support for deprecating exports.
Sep 24 2018, 12:15 PM

Sep 21 2018

DavidEichmann added a reviewer for D4953: Support for deprecating exports: DavidEichmann.
Sep 21 2018, 11:58 AM

Aug 30 2018

DavidEichmann updated the summary of D5059: testsuite: Save performance metrics in git notes..
Aug 30 2018, 10:10 AM
DavidEichmann updated the summary of D5059: testsuite: Save performance metrics in git notes..
Aug 30 2018, 5:36 AM

Aug 27 2018

DavidEichmann added inline comments to D5059: testsuite: Save performance metrics in git notes..
Aug 27 2018, 6:49 AM
DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..
  • Use average if previous commit has multiple recordings of the same metric (in testlib._collect_stats).
  • Remove TODO comments.
  • Update README.md
    • Remove 'Important Types' section. This is now self documenting.
Aug 27 2018, 6:46 AM

Aug 24 2018

DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..

Squash commits

Aug 24 2018, 1:14 PM
DavidEichmann added a comment to D5059: testsuite: Save performance metrics in git notes..
  • Output suggested text for the git comment message that will allow all the detected changes in performance tests.
      • removed optimisation to short circuit test if any metric fails (needed to give a complete suggestion)
      • added allow_changes_string()
      • check_stats_change takes now returns the change direction string.
      • use of TestRun.metrics changed (see comment above "self.metrics = []")
    • Use namedtuple PerfStat
      • check_stats_change takes PerfStat instead of individual arguments
Aug 24 2018, 1:09 PM
DavidEichmann added inline comments to D5059: testsuite: Save performance metrics in git notes..
Aug 24 2018, 1:06 PM
DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..
  • Use a namedtuple for performance stat objects.
  • Print a suggest git commit msg to make all performance tests pass.
Aug 24 2018, 1:06 PM

Aug 23 2018

DavidEichmann updated the summary of D5059: testsuite: Save performance metrics in git notes..
Aug 23 2018, 10:14 AM
DavidEichmann added inline comments to D5059: testsuite: Save performance metrics in git notes..
Aug 23 2018, 9:26 AM
DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..

Parse the git commit message for allowed changes in test metrics.

Aug 23 2018, 9:21 AM
DavidEichmann created D5091: Update integer_gmp_gcdext documentation..
Aug 23 2018, 6:27 AM

Aug 14 2018

DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..

Try catch 'git notes append'.

Aug 14 2018, 4:24 PM
DavidEichmann added inline comments to D5059: testsuite: Save performance metrics in git notes..
Aug 14 2018, 4:22 PM
DavidEichmann added a comment to D5059: testsuite: Save performance metrics in git notes..

I've incorporated the remaining changes from D3758. Some key differences

Aug 14 2018, 12:19 PM
DavidEichmann retitled D5059: testsuite: Save performance metrics in git notes. from Add perf_notes.py tool. to testsuite: Save performance metrics in git notes..
Aug 14 2018, 11:58 AM
DavidEichmann updated the diff for D5059: testsuite: Save performance metrics in git notes..

testsuite: Save performance metrics in git notes.

Aug 14 2018, 11:55 AM

Aug 13 2018

DavidEichmann added a comment to D5042: Correct limb length and assertion for gcdExtInteger.
  • Added more test cases.
Aug 13 2018, 6:44 AM
DavidEichmann updated the diff for D5042: Correct limb length and assertion for gcdExtInteger.

Correct limb length assertion for gcdExtInteger (fixes Trac #15350)

Aug 13 2018, 6:32 AM

Aug 10 2018

DavidEichmann added a comment to D5059: testsuite: Save performance metrics in git notes..

Note that this is mostly copied from D3758.

Aug 10 2018, 3:40 PM
DavidEichmann created D5059: testsuite: Save performance metrics in git notes..
Aug 10 2018, 3:38 PM

Aug 6 2018

DavidEichmann added inline comments to D5042: Correct limb length and assertion for gcdExtInteger.
Aug 6 2018, 3:29 AM

Aug 3 2018

DavidEichmann created D5042: Correct limb length and assertion for gcdExtInteger.
Aug 3 2018, 9:51 AM