Skip to content

Comments

fix -c -inline ddmd/mars.d#6657

Merged
dlang-bot merged 1 commit intodlang:stablefrom
MartinNowak:fix_doc_build
Mar 28, 2017
Merged

fix -c -inline ddmd/mars.d#6657
dlang-bot merged 1 commit intodlang:stablefrom
MartinNowak:fix_doc_build

Conversation

@MartinNowak
Copy link
Member

@MartinNowak MartinNowak commented Mar 28, 2017

  • fails when compiling because ddmd/mars.d imports ddmd.objc and the
    modulename != filename hack doesn't work for single file
    compilation, so ddmd/objc.d (instead of ddmd/objc_stub.d) gets
    imported and fails on a OSX-only symbol
  • fixed by always defining objc_initSymbols and adding it to the stubs

- fails when compiling b/c ddmd/mars.d imports ddmd.objc and the
  modulename != filename hack doesn't work for single file
  compilation, so ddmd/objc.d (instead of ddmd/objc_stub.d) gets
  imported and fails on a OSX-only symbol
- fixed by always defining objc_initSymbols and adding it to the stubs
@MartinNowak MartinNowak requested a review from CyberShadow March 28, 2017 16:10
@MartinNowak
Copy link
Member Author

While you're at it, may you have a look at this as well?

@CyberShadow
Copy link
Member

fails when compiling b/c ddmd/mars.d

Please avoid abbreviations in commit messages - in that context especially it can be confused by a path or identifiers.

Aside from that, this (DMD code and especially Objective C stuff) is really not up my alley... CC @jacob-carlborg

@MartinNowak MartinNowak added the Review:Blocking Other Work review and pulling should be a priority label Mar 28, 2017
@MartinNowak
Copy link
Member Author

Blocking proper release building (docs).

@CyberShadow
Copy link
Member

I'll enable auto-merge since this is a blocker.

@jacob-carlborg
Copy link
Contributor

LGTM 👌

@jacob-carlborg
Copy link
Contributor

Out of curiosity, why did this start to fail now?

@jacob-carlborg
Copy link
Contributor

BTW, I'm working on a better way to select if the Objective-C integration is enabled or not [1], although it seems difficult to come to some form of agreement.

[1] http://forum.dlang.org/post/2D143DD8-A978-4C60-9C78-488E19802B88@me.com

@dlang-bot dlang-bot merged commit e013038 into dlang:stable Mar 28, 2017
@CyberShadow
Copy link
Member

Strange, why is the commit CI status red? Travis seems to have failed but I don't see any failures on https://travis-ci.org/dlang/dmd/builds/216000505 ? Why did @dlang-bot merge it despite it being red?

@jacob-carlborg
Copy link
Contributor

The macOS builds where canceled?

@CyberShadow
Copy link
Member

Yeah, why?

@wilzbach
Copy link
Contributor

Quite simple, because those CIs aren't set as required yet and the bot only enforces the status of the required CIs. I have been pushing for setting all CIs except for CodeCov to required, but this has always been blocked with "our CIs have too many random failures and are unreliable"...

@CyberShadow
Copy link
Member

our CIs have too many random failures and are unreliable

Well, are they?

@CyberShadow
Copy link
Member

I think @dlang-bot should still wait for non-required CIs too, and not auto-merge if any CIs, required or not, have failures. If the non-required CIs have failures, a human can look at the failures, decide whether there's a problem, and merge manually.

@wilzbach
Copy link
Contributor

Well, are they

Hehe, I have done a couple of these statistics in the past. In short our infrastructure isn't optimal, so we do get some random connection timeouts and there are still random failures hidden in Phobos, but my point has always been that if we can't fix these issues how do we expect normal D projects to work? Moreover, by enforcement we can no longer ignore the fact that we have to eat our dog food from time to time.

I think @dlang-bot should still wait for non-required CIs too

I agree: dlang/dlang-bot#69

@MartinNowak MartinNowak deleted the fix_doc_build branch March 29, 2017 00:25
@MartinNowak
Copy link
Member Author

Yes, this was a slight design mistake we did when implementing the auto-merge feature. At that point there were a lot more failing/WIP CIs.

Out of curiosity, why did this start to fail now?

Think it's b/c we started to build ddmd docs (or maybe build them differently or so).

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

Labels

Merge:auto-merge Review:Blocking Other Work review and pulling should be a priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants