Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

The DRuntime makefiles changed to copy core.* D files as DI files. #218

Merged
merged 8 commits into from
Jun 26, 2012
Merged

The DRuntime makefiles changed to copy core.* D files as DI files. #218

merged 8 commits into from
Jun 26, 2012

Conversation

LightBender
Copy link
Contributor

The DRuntime makefiles will now copy all D modules in core.* instead of running DI generation on them, EXCEPT for core.sync.*

Required by: dlang/dmd#945

@alexrp
Copy link
Member

alexrp commented May 15, 2012

Looks good to me.

@LightBender
Copy link
Contributor Author

I have a waiting commit that will move thread.di to import/core and updates the makefiles accordingly, however, I need Zor's threading pulls merged before this can happen.

Requires: #214


copy: $(COPY)

$(IMPDIR)/core/%.di : src/core/%.di
Copy link
Member

Choose a reason for hiding this comment

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

The rule is redundant then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you are saying, I can remove the redundancy.

@MartinNowak
Copy link
Member

What's the reason for dropping headers?

@LightBender
Copy link
Contributor Author

The reasons are threefold. First, many of the D files are just interfaces for the CRT anyways. Second, because the pull that depends on this one will significantly change how DI files are generated and will strip out all function implementations that are not auto or template functions; which has the potential to destroy the semantics of some of the D files. Also Phobos does some CTFE into the DRuntime that is broken by the DI generation changes. This change protects those semantics. Third, many in the community feel that this is a better way of handling these files anyways as DI generation was never guaranteed by Walter to continue to function in it's present state, which is a rather broken file copy mechanism.

@LightBender
Copy link
Contributor Author

Please reference this thread for the pertinent conversations: http://forum.dlang.org/thread/op.weboiazv707hn8@invictus.skynet.com

$(IMPDIR)/core/%.di : src/core/%.di
cp $< $@

$(IMPDIR)/core/%.di : src/core/%.d
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave the file extension unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So: $(IMPDIR)/core/%.d : src/core/%.d ? Should work.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, but you would also need to change the extensions for the COPY targets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's done. I also update the rm/del commands as well.

@complexmath
Copy link
Member

Oops... this one was left long enough that it can't be merged.

@LightBender
Copy link
Contributor Author

How so? It passes the autotester and there is no beta yet ... As for it's effects, all it does is shuffle files around to make sure that CTFE continues to work in regards to DI generation, it doesn't change anything semantically.

@complexmath
Copy link
Member

I should have said that it can't be automatically merged any longer. I don't know why.

@LightBender
Copy link
Contributor Author

Ahh, i'll take a look at the recent merges to see what happened, I know that some of alexrp's stuff would cause a conflict with this one. Please expect a commit shortly, after I've tracked down the problem. This needs to get merged ASAP to clear the way for the DI patch.

@LightBender
Copy link
Contributor Author

Ok, rebased and merged, it should pull in correctly now.

@LightBender
Copy link
Contributor Author

Is there anything preventing this from being merged?

@complexmath
Copy link
Member

Can you rebase the pull request? GitHub is complaining that it can't be automatically merged. Sorry for the delay. I'll merge after it's updated.

@LightBender
Copy link
Contributor Author

Rebased again. It needs to be rebased every time someone changes thread.di because this pull moves that file from src\core to import\core.

@LightBender LightBender reopened this Jun 26, 2012
@LightBender
Copy link
Contributor Author

Sorry about that, clicked the wrong comment button.

@complexmath
Copy link
Member

FreeBSD 32 is listed as failing in the pull tester, but the detail suggests that the connection just dropped, not that there was an actual test error. I'm going to go ahead and merge.

complexmath added a commit that referenced this pull request Jun 26, 2012
The DRuntime makefiles changed to copy core.* D files as DI files.
@complexmath complexmath merged commit c813532 into dlang:master Jun 26, 2012
@LightBender
Copy link
Contributor Author

That's odd, might be a configuration problem on that tester, it's never failed FreeBSD32 before.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants