Skip to content

Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d#11515

Merged
dlang-bot merged 1 commit intodlang:masterfrom
MoonlightSentinel:the-bad-apple
Aug 19, 2020
Merged

Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d#11515
dlang-bot merged 1 commit intodlang:masterfrom
MoonlightSentinel:the-bad-apple

Conversation

@MoonlightSentinel
Copy link
Copy Markdown
Contributor

@MoonlightSentinel MoonlightSentinel commented Aug 5, 2020

They achieve the same behaviour as posix.mak simply forwards to build.d.

This is a workaround for https://issues.dlang.org/show_bug.cgi?id=20999
which seems to be caused by some local library based on these observations:

  • the failure is restricted to the macair host, no failures
    on D-Autotester.local
  • the failure happens upon the second invocation of build.d
  • removing --called-from-make makes the errror disappear.
    This switch enables a lockfile (generated/build.lock) which ensures
    that mutliple instances of build.d are run mutually exclusive (this
    was required during the make -> build.d transition but is obsolete now)

@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

A few observations:

  1. the failure is restricted to the macair host, no failures on D-Autotester.local
  2. the failure happens upon the second invocation of build.d
  3. Removing --called-from-make makes the errror disappear. This switch enables a lockfile (generated/build.lock) which ensures that mutliple instances of build.d are run mutually exclusive (required during the make -> build.d transition).

Maybe it's an issue within the local C library used by std.stdio.File?

@MoonlightSentinel MoonlightSentinel force-pushed the the-bad-apple branch 2 times, most recently from 3ad9d4f to f884d88 Compare August 7, 2020 10:09
@MoonlightSentinel MoonlightSentinel marked this pull request as ready for review August 7, 2020 11:20
@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

[Not ready for review, just curious if Azure doesn't pick up Draft PRs]

@MoonlightSentinel MoonlightSentinel marked this pull request as draft August 7, 2020 13:19
@MoonlightSentinel MoonlightSentinel marked this pull request as ready for review August 8, 2020 16:23
@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request and interest in making D better, @MoonlightSentinel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11515"

@MoonlightSentinel MoonlightSentinel marked this pull request as draft August 8, 2020 20:42
@MoonlightSentinel MoonlightSentinel force-pushed the the-bad-apple branch 4 times, most recently from 00fcb82 to cc7a03b Compare August 12, 2020 20:54
@MoonlightSentinel MoonlightSentinel marked this pull request as ready for review August 13, 2020 10:55
@MoonlightSentinel MoonlightSentinel changed the title Investigate the macair failure - remove lockfiles [WIP] Investigate the macair failure - remove lockfiles Aug 13, 2020
@dlang-bot dlang-bot added the Review:WIP Work In Progress - not ready for review or pulling label Aug 13, 2020
@wilzbach
Copy link
Copy Markdown
Contributor

Just commenting based on your commits:

  • there's no gdb on OSX (by default) as LLVM is the default
  • does the error vanish when -lowmem is removed?

@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

  • there's no gdb on OSX (by default) as LLVM is the default

Sorry, should've clarified that I hijacked this PR to check for the segfault when building d_do_test which I can't reproduce locally. OSX is currently irrelevant because the segfault only happens on Linux & FreeBSD.

  • does the error vanish when -lowmem is removed?

Maybe, didn't test that yet.

@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

Not sure why the auto-tester refuses to test this PR ...

@MoonlightSentinel MoonlightSentinel marked this pull request as draft August 13, 2020 18:42
@wilzbach
Copy link
Copy Markdown
Contributor

Not sure why the auto-tester refuses to test this PR ...

Typically there are either many other recently opened PRs or PRs with "auto-merge" which can't be merged due to other failures.
The preference logic of the auto-tester is roughly:

  1. tests new commits to master/stable
  2. auto-merge queue
  3. normal PRs by commit timestamp

Typically it's a PR being stuck in the auto-merge queue.

@MoonlightSentinel MoonlightSentinel force-pushed the the-bad-apple branch 2 times, most recently from eb1e0fe to c9e4718 Compare August 16, 2020 17:47
@MoonlightSentinel MoonlightSentinel changed the title [WIP] Investigate the macair failure - remove lockfiles Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d Aug 16, 2020
@MoonlightSentinel MoonlightSentinel changed the title Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d Aug 16, 2020
@MoonlightSentinel MoonlightSentinel changed the title Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d Use .NOTPARALLEL in posix.mak instead of lockfiles in build.d Aug 16, 2020
@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

Now using .NOTPARALLEL to avoid lockfiles. This still ensures mutually exlusive invocations of build.d unless someone manually start multiple instances of make at the same time (which should be hazardous for every Makefile)

@MoonlightSentinel MoonlightSentinel marked this pull request as ready for review August 16, 2020 23:28
Copy link
Copy Markdown
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a related note, having the parallelization of the test-suite fixed on OSX would be great. Also, this needs a rebase.

@MoonlightSentinel MoonlightSentinel removed the Review:WIP Work In Progress - not ready for review or pulling label Aug 18, 2020
They achieve the same behaviour as `posix.mak` simply forwards to build.d.

This is a workaround for https://issues.dlang.org/show_bug.cgi?id=20999
which seems to be caused by some local library based on these observations:

- the failure is restricted to the macair host, no failures
  on D-Autotester.local
- the failure happens upon the second invocation of build.d
- removing `--called-from-make` makes the errror disappear.
  This switch enables a lockfile (`generated/build.lock`) which ensures
  that mutliple instances of build.d are run mutually exclusive (this
  was required during the `make` -> build.d transition but is obsolete now)
@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

Rebased.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants