dlang / project-ideas

Collection of impactful projects in the D ecosystem
36 stars 12 forks source link

Merge druntime into the dmd repository #92

Closed maxhaton closed 1 year ago

maxhaton commented 2 years ago

Description

dmd and druntime are the same project. They should be developed as such. The current separation currently leads to confusion, and slows development progress because every change in effect has to go through CI, conceptual review, etc twice

Distinction between this and a traditional monorepo: Merging dmd and druntime into a new project is better than nothing, but fundamentally the runtime is a subproject of the compiler.

gdc and ldc both use this structure already.

What are rough milestones of this project?

  1. Merge them while maintaining history
  2. Automatically update the old repo
  3. Fix upstream stuff that breaks.

How does this project help the D community?

it will smooth the process of developing D.

Recommended skills

good knowledge of git

What can students expect to get out of doing this project?

Point of Contact

## References
Geod24 commented 2 years ago

Note: We already had quite a few discussions about it. Major stakeholder were @CyberShadow , @PetarKirov . The consensus was that we should preserve history, so merging is a must, and git allows this. @ibuclaw has been experimenting with this IIRC.

ibuclaw commented 2 years ago

Note: We already had quite a few discussions about it. Major stakeholder were @CyberShadow , @PetarKirov . The consensus was that we should preserve history, so merging is a must, and git allows this. @ibuclaw has been experimenting with this IIRC.

Yes.

Geod24 commented 2 years ago

So what's the next step ?

maxhaton commented 2 years ago

Politics

CyberShadow commented 2 years ago

I wrote a script to do it shortly after that meeting, but I can't find it. I guess that's not important if ibuclaw has another implementation.

ibuclaw commented 2 years ago

I wrote a script to do it shortly after that meeting, but I can't find it. I guess that's not important if ibuclaw has another implementation.

A short script pretty much is what I did as well. Nothing fancy, just git clone/remote add/fetch/filter branch.

Geod24 commented 2 years ago

After discussing with @CyberShadow and @ibuclaw , I've set up up the date as 2022-07-09, 13:00 GMT. Invite has been sent to known stakeholders and another announcement will be made on Github.

EDIT: https://github.com/orgs/dlang/teams/team-dmd/discussions/1

CyberShadow commented 2 years ago

A short script pretty much is what I did as well. Nothing fancy, just git clone/remote add/fetch/filter branch.

Hmm, I thought we decided to not use filter-branch, but git-subtree, to preserve commit IDs and allow rebasing.

CyberShadow commented 2 years ago

When did we discuss this? Was it last year? Maybe knowing the date could help me find my notes on it.

ibuclaw commented 2 years ago

When did we discuss this? Was it last year? Maybe knowing the date could help me find my notes on it.

My memory is we discussed it alongside what to do with bugzilla migration.

ibuclaw commented 2 years ago

A short script pretty much is what I did as well. Nothing fancy, just git clone/remote add/fetch/filter branch.

Hmm, I thought we decided to not use filter-branch, but git-subtree, to preserve commit IDs and allow rebasing.

Just looked it up, and I in-fact used filter-repo in my test repository.

CyberShadow commented 2 years ago

Just looked it up, and I in-fact used filter-repo in my test repository.

Right, that's the same concept (different implementation). I think the trade-off we discussed was whether we want retroactively linear history (in which case we rewrite the history with something like those tools) or we do a subtree merge which just adds a commit that glues the two histories together. I think we decided to do the latter to preserve commit IDs.

Though, I haven't checked, maybe your tool is using git-filter-repo just to do the subtree merge operation for every branch? I did not think there would be too many branches to require using such a tool but maybe I'm wrong.

ibuclaw commented 2 years ago

Just looked it up, and I in-fact used filter-repo in my test repository.

Right, that's the same concept (different implementation). I think the trade-off we discussed was whether we want retroactively linear history (in which case we rewrite the history with something like those tools) or we do a subtree merge which just adds a commit that glues the two histories together. We decided to do the latter to preserve commit IDs.

Though, I haven't checked, maybe your tool is using git-filter-repo just to do the subtree merge operation for every branch? I did not think there would be too many branches to require using such a tool but maybe I'm wrong.

The only branches that matter are stable and master. All else can be ignored. Were tags discussed too? i am half-minded just to list all druntime tags+commit numbers, then re-tag them with a druntime- prefix.

mdparker commented 2 years ago

@CyberShadow As best as I can tell, we had the repo merge meeting in either in November or December. It was separate from our monthly meetings. The monthly meeting in December was where we discussed the Bugzilla to GH migration. The repo meeting was either a week or two before or after that.

mdparker commented 2 years ago

Okay, looks like it might have been the first week of November. I've found a reference to it in an email I sent to Max. It must have been November 4.

CyberShadow commented 2 years ago

Thank you! I found my draft script:

#!/bin/bash
set -eEuo pipefail

mv dmd dmd-"$(date +%s)" || true

git clone ~/work/extern/D/dmd dmd

cd dmd

mkdir compiler
git mv {changelog,ci,docs,ini,samples,src,test} compiler/
git commit -m 'Move DMD files into compiler/'

git fetch ~/work/extern/D/druntime
git merge -s ours --no-commit --allow-unrelated-histories FETCH_HEAD
git read-tree --prefix=druntime/ FETCH_HEAD

git commit -m "Merge druntime"

@ibuclaw Is yours similar?

CyberShadow commented 2 years ago

BTW as per https://github.com/orgs/dlang/teams/team-dmd/discussions/1 I think we should not proceed until the LDC team is on the same page with us.

ibuclaw commented 2 years ago

@ibuclaw Is yours similar?

I would like to move dmd into a compiler directory, but that will probably break a lot more stuff than just leaving it where is for now.

So let's start with

src/dmd
src/druntime

and any future reshufflings, we'll deal with that further down the line.

CyberShadow commented 2 years ago

Hmm, that's not the consensus we reached at the meeting. I recall we discussed all such options at length, and there may have been arguments against this approach which I don't remember now.

I think we need to discuss and plan this some more.

ibuclaw commented 2 years ago

I think we should not proceed until the LDC team is on the same page with us.

druntime tree (annotated our commits with !)

* !9c0d4f91 (HEAD -> stable, tag: v2.100.0-rc.1, tag: v2.100.0, upstream/stable) fix Issue 23046 - [REG][CODEGEN] __simd(XMM.LODLPS) bad codegen
* !e361d200 Mark gcRanges() with return attribute
*   !27834edb (tag: v2.100.0-beta.1) Merge remote-tracking branch 'upstream/master' into stable
|\  
| * !48dea88d Fix issue 22763 - importing std.utf fails in BetterC
| *   !1cb74998 Merge pull request #3801 from ljmf00/add-nogc-asserterror
| |\  
| | * !a4bc2c0e core/exception: annotate @nogc on AssertError ctors
| |/  
| * !1d92f8f1 Add `scope` to __aaGet functions

ldruntime tree

*   44d72bb93 (tag: ldc-v1.30.0-beta1) Merge remote-tracking branch 'dlang/stable' into ldc-merge-2.100
|\  
| * !9c0d4f914 fix Issue 23046 - [REG][CODEGEN] __simd(XMM.LODLPS) bad codegen
* | 6cedc1c8d LDC: Add missing symbols in importC's __builtins.di
* | d3047ba07 LDC: Define __va_list in importC's __builtins.di
* | 28dbc7289 rt.sections_android: Add newly required `return` attribute
* | 62c943bc1 Merge remote-tracking branch 'dlang/stable' into ldc-merge-2.100
|\| 
| * !e361d200b Mark gcRanges() with return attribute
| *   !27834edb5 Merge remote-tracking branch 'upstream/master' into stable
| |\  
| | * !48dea88d8 Fix issue 22763 - importing std.utf fails in BetterC
| | *   !1cb749981 Merge pull request #3801 from ljmf00/add-nogc-asserterror
| | |\  
| | | * !a4bc2c0e4 core/exception: annotate @nogc on AssertError ctors
| | |/  
| | * !1d92f8f1e Add `scope` to __aaGet functions

I don't see what would prevent them from continuing to merge as per their current status quo if we use subtree.

ibuclaw commented 2 years ago

Hmm, that's not the consensus we reached at the meeting. I recall we discussed all such options at length, and there may have been arguments against this approach which I don't remember now.

I think we need to discuss and plan this some more.

We can switch up as needed, but did we really want to break all open dmd and druntime PRs? Druntime is easier to manage the fire because the number of open PRs is relatively low.

CyberShadow commented 2 years ago

Have you checked if moving files to a subdirectory will break PRs?

CyberShadow commented 2 years ago

I think that even if it will break GitHub's merge, git itself should be able to handle it in a rebase.

ibuclaw commented 2 years ago

I think that even if it will break GitHub's merge, git itself should be able to handle it in a rebase.

I can do a sanity check then. But even so, then it won't matter if we move it later then (post-archiving druntime)? i.e: it can be done same day, but just not to be conflated with the merging of trees itself.

There'll be quite a bit of shuffling druntime around post merge anyway.

CyberShadow commented 2 years ago

Maybe, I think the main problem will be that the repository is going to be a confusing mess until it's fully sorted out. Things like CI scripts (and documentation, if the span is sufficiently long) will have to be adjusted more than once.

maxhaton commented 2 years ago

When it comes to breaking PRs it's probably worth making the distinction between breakage that a machine (git) will struggle with versus breakage that the machine and a human can't fix without too much effort (e.g. able to circumvent git entirely etc.).

ibuclaw commented 2 years ago

Maybe, I think the main problem will be that the repository is going to be a confusing mess until it's fully sorted out. Things like CI scripts (and documentation, if the span is sufficiently long) will have to be adjusted more than once.

For transparency, rough outline (script) is here, complete with patches for merge-stable-master and post-repo-merge fix-ups.

https://github.com/ibuclaw/dmd-monorepo-scripts

atilaneves commented 2 years ago

Why not druntime and phobos?

ibuclaw commented 2 years ago

Why not druntime and phobos?

The compiler doesn't have a dependency on phobos (and if it does, then that needs fixing).

atilaneves commented 2 years ago

Right, but the runtime is required... at runtime, which is why its code is bundled up with Phobos.

ibuclaw commented 2 years ago

Right, but the runtime is required... at runtime, which is why its code is bundled up with Phobos.

To put it another way, swapping two parameters around in Phobos doesn't break the compiler. As the code that the compiler generates is not bound to the ABI of Phobos, but it does matter what the ABI of Druntime is.

It happens often enough that there's two PRs - one in dmd, one in druntime - both on their own break all pipelines for each other, but both need to be merged in order to get things working again.

Geod24 commented 2 years ago

We never have to get PRs into Phobos and DMD at the same time. Not even Phobos and druntime. It's also DMD or druntime first, then Phobos. But for druntime it's not infrequent.

Geod24 commented 1 year ago

Done

ibuclaw commented 1 year ago

:tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada::tada: