-
-
Notifications
You must be signed in to change notification settings - Fork 606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make lib a package #9844
Make lib a package #9844
Conversation
Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
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 referencesYour 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 locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#9844" |
f0e6391
to
badd75e
Compare
No, we're not packagizing dmd. |
Then we're not utilizing D. Packaging the repository will make it easier to understand DMD, and will also allow us to utilize D's encapsulation features. Some parts of DMD do not need to be publicly exposed, but do need to be shared between modules. Packaging will help encapsulate that, expecially when using DMD as a library. |
badd75e
to
76e7660
Compare
|
||
version(Windows) {} | ||
else version(OSX) {} | ||
else: | ||
package(dmd.lib): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WalterBright There's value in this ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep thats the entire point of the PR.
Sources sources = { | ||
frontend: | ||
dirEntries(env["D"], "*.d", SpanMode.shallow) | ||
packages.map!(p => env["D"].buildPath(p).dirEntries("*.d", SpanMode.shallow)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would just "{,lib}*.d"
work?
See also: https://dlang.org/library/std/path/glob_match.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
For additional awesomeness, a follow-up PR could be to build the lib
package separately for faster build times (:
(though definitely not required here)
Definitely, I want this PR as simple as possible though. |
I no longer have a say in this (cc @atilaneves), and am neutral on the matter; I consider it minor and not worth fighting over. But allow me to share a thought. Years ago I joined this project at Facebook: https://github.com/facebook/hhvm. It's daunting. Two million lines of code (now it's closer to three), scantily documented (that has improved quite a bit), sprawled over an arcane directory structure, with an awkward build and test process. To add insult to injury, the coding convention used prescribed a 2-space indent size and a maximum line length of 80 columns. Several directories in the project contained over one hundred files; see e.g. https://github.com/facebook/hhvm/tree/master/hphp/runtime/base (153 files), https://github.com/facebook/hhvm/tree/master/hphp/util (220 files), or https://github.com/facebook/hhvm/tree/master/hphp/runtime/base (a whopping 367 files, and where the team spent most of its time). That's the case across languages, too, e.g. https://github.com/facebook/hhvm/tree/master/hphp/hack/src/server has 137 ML files. The first day I joined the project I could have found a dozen ways to reorganize the project to better suit my preferences. Yet I didn't. Why? Because it was the choice of the project leader. This kind of stuff does not matter to a professional, and basic courtesy and collegiality leaves such matters to the project leader. I got on with the project and a good time has been had for one year. @WalterBright opposed major reshuffling of dmd's directory structure. He is the project leader and by far the most prolific and longstanding contributor to the project. It is professional courtesy to let him have his say in such matters, and repeatedly attempting to undermine his decision is disrespectful. There has been this implication that if only this one more grand in scope but trivial in substance restructuring would be done, a lot of latent power would be unleashed and proposers of said restructuring would go on and do wonderful things in the codebase. Having lived through several such "grand but trivial" restructurings with @WalterBright, I can say there's plenty of evidence that the best thing about such a restructuring is it creates precedent for the next restructuring, in an endless cycle of busywork. Encapsulation is not even a win of this PR, as #9852 shows that most symbols can in fact be made So, I think the right thing to do is go forward and do great work. Please be respectful and do not reopen the matter. A directory structure flatter than what one would prefer does not prevent a professional from getting work done. |
To accuse me of attempting to undermine him is grossly disrespectful when you asked for the exploration of when we meant by what we were saying in the linked thread. It is literally the top quote of the linked comment!
Of course, I started with this precisely because it is the only part of the codebase that is actually encapsulated properly (to within trivial epsilon). The rest of the codebase is not ready for such transformation, and I accepted that. Thats why I've been trying to sparsify the import matrix.
Of course, the DMD codebase is a convoluted, maldocumented pile of cluttered
That is plainly not true. Yes
Thats nice, but I was aiming for a minimal diff. The last one Walter objected to was much larger and did a bunch of renaming and other unnecessary (from the point of atomicity) code motion and I could accept rejection of that PR on those grounds.
The fact that this PR compiles (modulo merge conflicts) tells you that those few functions are called only from
So, should I rebase...
... or should I not. And please don't insult me like that, you are above that kind of behaviour.
Maybe, but it does slow them down. But the main impetus for properly structuring the DMD codebase is not for "the professionals", its for those who are less familiar with the codebase. Every new contributor to DMD starts out not in that category, including people who are very much professionals. If those people look at the codebase and make a business decision "do I invest the time to figure out where the bug I wish to fix is, or do I work around it?", the current state of the organisation of the codebase is not a motivating factor for fixing the bug. And I've seen this happen to very prominent people within the D community. |
Sorry for assuming. The way I saw this done was make By my count #9852 makes 132 symbols private and leaves a residue of 13 symbols accessible. This PR makes 142 symbols package-level (a weaker form of protection) and leaves 4 public symbols accessible. That includes |
That napkin math also leaves out that it removes 9 files (for the price of a folder) from the main source directory which, while not particularly spectacular when compared to 125 (plus headers), will be significant when it starts to accumulate to make navigation much simpler, particularly for new/potential contributors, Walters oddities with More to the point, if this is not ready now, what could possibly change in the future that would alter it? |
That's not the only win (!), we also get better structure and documentation which is again one of the top 3 reasons people don't contribute to DMD (or use it as a library). Just ask anyone on Slack on the NG about their experience with working with DMD. Not being able to find things is a huge problem for everyone. |
That's a question for @WalterBright. My understanding is that he wants a flat directory structure, and that he believes a hierarchical package structure in and of itself would not bring any significant improvement. I'm neutral with regard to flatness (would work with any structure the project manager chooses), and I do agree with him that switching to a more hierarchical structure is low-impact. If I were to opine on what the right thing to do is, there'd be a concerted work on better information hiding (high-impact stuff that currently is non-existent), from which repackaging would come as a natural consideration. The whole argument centered on navigation is very weak.
I think what's a huge problem for everyone is that a compiler for a real programming language is a complex program. By my experience we're deluding ourselves if we believe that someone would be entirely capable of contributing to a compiler yet they're unable to navigate 125 files in 1 directory, and 10 directories with 12-13 files each would be the enabler. |
That was rhetorical.
Navigation has more facets than just draining the mental energy of people who know the codebase well, scrolling through a list, it affects:
For example, when I started, knowing nothing about compilers and having not done any CS (aside the bastardisation that is taught in industrial engineering), getting dcompute up and producing code was a baptism by fire of the DMD, LDC, LLVM & Khronos' SPIR-V codebases, there is no other way to describe it. Working with DMD was a right pain in the rear end, so much so, that it was usually faster for me to look up the corresponding concept in clang and figure out what it was doing and what other things were related to it and then deduce where it was in dmd, because it was so much easier to navigate (and the documentation so much better its not even funny). And that was in C++(!!!!). For a long time it was always much easier to look at the headers DMD keeps around for LDC & GDC to find things because everything was in one place and compact (no function bodies) and the files were appropriately named. I think you severely underestimate the impact creating packages would have.
You are technically correct, but alas capability is not the factor of interest here, (percieved) cost of effort is. Very capable people that I have spoken to, have decided it is not worth the effort to fix a compiler bug themselves because it (appears to be) too unorganised and they'd rather sub optimally work around the problem because they have more important things to do with their time. On the other extreme is those who lack experience having something that is easy to understand the structure and components and layout makes a HUGE difference. If DMD had been as nicely organised as clang I could have probably saved myself at least two weeks getting up and running. There are serious opportunity costs with the status quo, don't blithely dismiss them just because you are familiar with the ins and outs of the codebase and its been a while since you had to learn them. |
For another tack - there are many ways to help newcomers with the project:
These are obviously good to have and encouraged by the project leader. If the purpose is to make life better for newcomers, why get stuck in one argument instead of working on the many other very productive opportunities? |
Doesn't help if the organisation of the documentation (which follows the organisation of the code: see LHS margin of https://dlang.org/phobos/dmd_access.html) is still terrible. More is nice but it doesn't help me find the thing I'm looking for if I don't find it the first time around. This is a quality vs quantity thing.
Once I have found what it is I want, that is somewhat useful, although I could grep to see where and how it is used which would probably tell me more about what I want to know and how to use it. Unlike Phobos which is a library and thus examples are key to documentation, DMD is an application, functions and classes exist to be used and used they are.
Those are good, they help lessen the cognitive burden on a file granular scale, but they don't help discoverability or navigability or cognitive burden on a project wide basis (like what files do I look in for the thing that does X?).
Because it is front and centre to everyone who has anything to do with the codebase, especially newcomers. You open up An example / thought exercise: suppose you want to write a tool that given a module declaration warns if any statement in a function/method contain more than X expressions (as a crude measure of the per line complexity of the code). Which classes/functions would you use and for what purpose? Time how long it takes you using just the source code and https://dlang.org/phobos/index.html and other pages under https://dlang.org/phobos/ , no asking others for help, no looking at #7639 etc. Then think back on how long it would have taken you to do that if you had packages to guide your search and then extrapolate that experience to someone less informed (with D, compilers, software engineering etc.) than yourself. I'll post a answer after you post one. |
Would a better readme help? |
Obviously it can't make it worse, since I don't think you could write a less useful one without being actively deceitful. But it would at best paper over the chasm. |
Please keep this in mind when evaluating PRs removing the header files from the backend. I have also recommended public functions in a module be placed "above the fold" with the private leaves below (the opposite of the typical C/C++ source file). Unfortunately, this breaks the file history, so I'm reluctant to do it unless working on a particular piece of code anyway. Having a public/private fold makes for easier discoverability. |
It would be great to write a TableOfContents.md that lists each source file and what's in them. They can even be grouped into hierarchical categories in the list. In fact, the Ddoc header for each source file should illuminate what the module does, but such is absent from most of the files. A great start would be to add such to each file. Then it could be cut/pasted into the TableOfContents.md. |
Also because GitHub displays the readme below the files you have to scroll down to 'r' to even know there is a readme rendered at the bottom. People with a random / casual / passing interest will see the wall of files, they may well move on before they even see / know that the readme is there. |
@WalterBright has made his preference known so I think we should all accept that and move on. Personally I use DCD to find where definitions are so file structure doesn't matter to me in the slightest. I agree with @andralex that there's no evidence that changing the structure would attract more developers. Compilers are complicated; most people won't want to or be able to work on one. Better documentation would probably be a better use of one's time if the goal is to attract contributors. |
That presumes you know what symbols you are looking for... perhaps you'd like to participate in the challenge I gave Andrei?
That is the wrong way around. Contributors come when they come (unless we seriously up out marketing, and I don't know about the rest of you, but I'm not a marketing person), so the trick is not intentionally having a light to wave to attract attention, its to not look like an anglerfish when they do find that light. The goal is to not scare then away when they do show up, and the malorganisation of the DMD codebase does a pretty good anglerfish impression (completely irrespective of the quality and comprehensiveness (or lack thereof) of the documentation). |
Not really, no. It's a moot point anyway. |
I would like to point out that I have not seen a single good argument of why the existing structure is good. I have only seen two arguments: A. Walter's tools don't work with subdirectories. He's mentioned B. This is the current structure and the changes don't have high impact. I see this argument quite a lot for the core projects. If a change doesn't cure cancer, solves world hunger an peace on earth it's not worth doing. I would like to point out that most of the existing code, especially in DMD, has not gone through the same code reviews that we have now. I can say that most code in DMD would not pass code review on my watch. |
I humbly submit there may be more of a need to move on, than what you'd consider a good argument. We really need to collectively stop bickering about this stuff. |
Personally, I gave up 3 years ago after this conversation. Like the overwhelming majority of contributors (and by overwhelming, I mean everyone but Walter), I see value in packaging, but it is Walter's rightful prerogative to refuse a change no matter the justification. |
Indeed i can do no better than to defer to #9511 (comment)
@WalterBright what are those tools (and would they be useful to the rest of us for day to day work)? It seems the best forward would be to make the tools work better. |
Another reason lib specifically, should be a package is so that everyone can ignore it more easily when they go about their DMD business (including with your tools): The only functional (and thats stretching it) commits post conversion to D(!) (c.a. ~4 years ago) are:
|
I have come to the realisation that far from being a frontend package, All of the points about packaging still stand, and none of this reflects well on you. When trying to attract new developers you should be careful to not lose the ones you already have... |
Eh? |
No, we're not letting this go. This is a OPEN SOURCE COMMUNITY, not a tyranny.
Be that as it may be, but it's our right to fork DMD and/or just drop dmd and focus LDC. This is sth. that will happen if this silliness continues. Without a community or contributors, dmd is dead. How am I supposed to motivate students to invest their time here (or my own for that matter) if we waste it with such silly walls? |
If I could tactfully interject... From my perspective this code is as stale (for want of a better term) as the backend and as far as LDC is concerned doesn't exist except as a rule in the merge scripts to discard. Wether it ends up as a package in the backend is none of my concern, I mean it probably should but the backend is such a mess anyway that is doesn't make much difference (and I don't care for it). I closed this, not because I don't think DMD should be packaged (it most definitely should), but because this doesn't belong in the frontend at all. So in its current state, quite aside the fact this can't merge due to a merge conflict, this PR doesn't make sense. If you wish to do the honours and make the PR to relegate this to the backend, be my guest, but I'm going to sleep and will do it tomorrow if it hasn't been done. |
Nothing in the backend calls any of the functions in the lib/scan code, and vice versa. Putting it there would make things awkward, as it does import root. Does ldc offer a way to read libraries directly generate libraries directly? Doing that is part of what makes dmd fast, it doesn't need to write a blizzard of files to generate a library. |
You can pass it |
I expect that, all compilers do it since Datalight C innovated that in the 1980's :-) DMD does do things that AFAIK no other compiler does:
The DMD makefiles used to use this, but at one point the makefiles were redone to use a librarian, I have no idea why that was done. |
|
I thought the entire point of root. Also I forgot to mention this is also completely unused by GDC as well. Think of this as manual DCE. |
|
Wby can’t the backend use |
~~Perhaps this can live in a |
The duplication isn't exact, meaning there'd have to be some rework. Frankly I thought it wasn't worth the bother. I've been more concerned with reworking other parts of the backend to make it more tractable, when I have some time. |
It is quite clear that this PR is dead in the water, however if I were to politely intrude in the conversation and point out a few things. @WalterBright is currently the BDFL for the d language, so technically speaking it is a tyranny here. Correct me if I am wrong about this, but It seems like Walter here is not against this in principle, but rather it causes him unpleasant developer experience with regards to his work flow when coding for dmd. Which it needs to be address if this PR were to has any chance of accepting. |
#9511 (comment)
cc @andralex @wilzbach