ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.58k stars 396 forks source link

Dune init proj should create a .gitignore containing _build #9567

Open yawaramin opened 7 months ago

yawaramin commented 7 months ago

Desired Behavior

dune init proj should create a .gitignore file as part of the new project. The .gitignore file should contain _build.

Related: #207

Alizter commented 7 months ago

What happens if .gitignore already exists?

We could probably add some logic that checks for an existing .gitignore and adds the _build entry if not included, but that is a little tricky when trying to compare _build/, _build/**, etc.

yawaramin commented 7 months ago

When dune init proj is run, it creates a new directory for the project files. There won't be a .gitignore file in a newly-created directory.

yawaramin commented 7 months ago

Another idea–let's also add an empty .ocamlformat file to the initial project to enable formatting it, since ocamlformat won't work unless this file is present in the project.

chshersh commented 7 months ago

Yes to both default .gitignore and .ocamlformat 🙏🏻

Having a fully-configured project from the start enables you to focus on relevant parts. I can't tell enough how important it is!

Leonidas-from-XIV commented 6 months ago

@yawaramin It does not if dune init is run with . since #9447 was merged.

toastal commented 6 months ago

As someone using darcs + topiary for VCS + formatting respectively, I don’t know that these files should be automatically generated without a consenting flag, or a CLI [y/N]?, etc. …Or at least some logic that checks if there is a .git directory first. Something smells funny about endorsing specific tools which further ingrains status quo & adds an implicit endorsement for said tools therein.

toastal commented 6 months ago

If I were (hand-wavingly) designing this, at least on the VCS system, I would check first for .git, .hg, _darcs & provide the appropriate ignore/boring files otherwise ask the user if they would like a file generated.

yawaramin commented 6 months ago

These defaults will be a massive win for the vast majority of users who are using git and the OCaml Platform recommended ocamlformat. Also I don't think we need to tip-toe around the fact that git is the de facto VCS of the OCaml ecosystem.

If you are a darcs and topiary user, these files will have absolutely zero impact on your project, aside from adding a few bytes of overall file size, unless you notice and delete the files before checking in. Which I am fairly sure darcs and topiary users are smart enough to do.

(Also, isn't darcs a dead project and its successor Pijul taking over?)

toastal commented 6 months ago

Crap. This doesn’t work now that I think about it & happened to me again today. dune--unlike git, darcs, pijul, nix, npm, etc.--does not init in the $PWD but creates a new directory (which I then have to cp files/folders out of (which my fish history shows I’ve done several times)). If it were created in the same directory it could at minimum skip creating a .gitignore if .hg, .pijul, or _darcs directories exist, but not even dune init project foo . works for present working directory.


Neither I or anyone else will know if git and ocamlformat will maintain hegemony in the next decade. Would dune be creating svn:ignore a decade ago? When would it decide it was time to switch to git? When another tool inevitably usurps Git like the VCSs of old, will dune join that tool & leave Git behind or continue to maintain while echoing ignores for both or just one? At what critical mass? (Will ocamlformat ever support tabs? 😅)

Rather than assuming the tools of today will be the tools of tomorrow, I feel it’s a safer long-term bet to check+consent. Setup is a one-&-done task, so adding a prompt shouldn’t be seen as annoying, but explicit, future-proof, extendable, & easier than running set -x PROJ "foo" && dune project init $PROJ && rm $PROJ/.{gitignore,ocamlformat} when those tools come (or now if you are me).


darcs dead tangent

No, they are independent projects & darcs is still maintained+developed by a Haskell team--they even respond relatively quickly on IRC. I recently spent around 1½ month trying Pijul so I could assess if it was the future & there are a lot of edge cases that lead to darcs being the better tool for my workflow even at the cost of some performance loss (which I never got to the point of noticing)--with the biggest being, there isn’t a rebase to fixup or do WIP commits.

Of note tho, is that darcs is officially supported by both OPAM and Nix where Pijul isn’t yet supported.

yawaramin commented 6 months ago

dune--unlike git, darcs, pijul, nix, npm, etc.--does not init in the $PWD but creates a new directory

Fixed since #9447

Neither I or anyone else will know if git and ocamlformat will maintain hegemony in the next decade.

We are not making an irreversible decision here, let's worry about the software of the next decade if and when it actually happens.

adding a prompt shouldn’t be seen as annoying

Prompts are annoying though, if I tell dune 'make a new project for me', I don't want it to check with me on every single trivial decision, I just want it to do the right thing automatically.

toastal commented 6 months ago

I’m very glad to see that fix!

With that fix, would anyone be offended if logic were at least added to check the others first? While slightly miffed, I can understand at present wanting .gitignore but I don’t think it’s the right thing to do in the case of .hg, .pijul, _darcs--where their appropriate files are echoed (Pijul’s .ignore, darcs’s .boring, not sure about Mercurial). I’m willing to volunteer to help on such a task.

yawaramin commented 6 months ago

Personally I don't think dune should have logic to check every possible VCS (what about Fossil?); imho it's not worth the effort to add so much checking to avoid creating a single .gitignore file with a few bytes in it that can immediately be deleted, but ymmv.

toastal commented 6 months ago

For a just few more bytes you could echo out the ignore files for all VCSs! If the match case is implemented, adding new ones following the previous template would be a cakewalk.

Good defaults are one thing, but doing unexpected things isn’t a good UX. A Fossil user shouldn’t find unexpected ignore files, nor should they be made to feel inferior for choosing a tool different than dune’s choice. This makes the barrier to other VCSs feel worse since they now need to do extra steps—which is hard enough as it is to get folks to try anything that isn’t Git.

yawaramin commented 6 months ago

First of all, it's not the goal of this issue to make anyone feel inferior or locked out. Let's put this in perspective–we're talking about a couple of tiny, plaintext files that have literally no impact on those users and can immediately be deleted. Any reasonable person should be able to agree that it's a complete non-issue. I can understand objecting to a decision or unexpected behaviour which has a material negative impact on usability for those users, but applying such a high bar here seems like making a mountain out of a molehill.

Also, the suggestion is not new or unprecedented–plenty of project setup tools create .gitignore files already.

Also, how about just giving it a minute? We are talking about a feature that hasn't even landed in dune yet. If and when it does, there can of course be a discussion about generalizing it if there is demand. And it's not clear to me that that there is. Again, dune is not set in stone, if something is needed in the future, it can be added.

Finally, by your own argument, we don't know if dune will maintain hegemony in the next decade. I might argue that chances are higher for git than they are for dune. So, we don't even know if the energy spent on this is warranted.

toastal commented 6 months ago

To be clear, I think adding generic “ignore file support” is a admirable, helpful goal. Where I disagree is in the implementation—specifically, naïvely injecting the improper, unexpected file for non-Git VCSs, where their detection is a simple filesystem folder existence check & a match-case away. Tiny or not, it can be both a nuisance & wasted bits while degrading the developer experience for those affected.

I have already offered to help or do said check (even if other ignore files were to be considered out of scope).

yawaramin commented 6 months ago

So, should we stop at VCS? Because Docker is pretty popular, and if there is a Dockerfile in the directory, should we then add a .dockerignore? The argument can go for many tools :-)

EDIT: I disagree that it 'degrades' the developer experience (how is an extra few bytes that are completely ignored by the toolchain, a degradation).

toastal commented 6 months ago

Moving the discussion out of the two proposed files isn’t really helping.

You can do negative checks to see if non-Git VCS is used & then don’t echo a .gitignore—this is a bare-minimum improvement to DX (considering OPAM supports hg & darcs, as does a lot of other OCaml tooling seem to at least minimally support).

toastal commented 6 months ago

Sorry, I’m definitely mixing up the words since developers are the users. I mean the UX. A good user experience for developers is a tool doesn’t do unexpected or unnecessary things--or put the other way, does the right thing™.

yawaramin commented 6 months ago

We agree that a good DX is a tool that just does the right thing. I just think that a simple, best-effort approach to cover like 99% of developers (git) is acceptable, as the tradeoff for a simpler implementation. I understand that ideally dune can check for a bunch of VCSs and generate ignore files for them all, but is it worth the additional complexity to target a handful of developers?

Anyway, code talks. If you are asserting that it's really not that complex to implement this, then you could possibly just implement it and send a patch, and let people take a look at it.

toastal commented 6 months ago

patch

What address can I send a patch to? Microsoft GitHub will let you attach all sorts of stuff like Microsoft Word Document files to comments, but not a useful type like .patch or .diff files.

nojb commented 6 months ago

What address can I send a patch to? Microsoft GitHub will let you attach all sorts of stuff like Microsoft Word Document files to comments, but not a useful type like .patch or .diff files.

Note that you can append a .txt extension to your file and then Github will let you attach it.

Having said that, for reviewing purposes a PR is better, unless the amount of code is really small.

yawaramin commented 6 months ago

Yeah, sorry for the loose wording, I really just meant a PR.

toastal commented 6 months ago

I have something 90% done & working (sans tests). The last hurdle is getting over the hump for “what if file exists?” that someone asked about, which “append to it” seems the correct answer for ignore files, but trying to thread that needle isn’t going as expected where types line up, but appending not happening.

toastal commented 6 months ago

I WIP diff is here https://github.com/gabyfle/dune/compare/feature/gabyfle/gitignore-fmt-init...toastal:ignore-plus-plus?expand=1 on Microsoft GitHub’s Git forge

toastal commented 6 months ago

(going to bed… feedback very welcome as I’ve only dabbled in OCaml with no experience in an existing codebase)

toastal commented 6 months ago

Didn’t have time yesterday, but found the missing ?append I forgot to thread.

I have opened a pull request with personal suggestions via Microsoft GitHub issue #9567 as a separate merge request against this branch; see https://github.com/gabyfle/dune/pull/1

This patchset would suggest:

  1. Skip creating .ocamlformat if the binary is missing rather than warning & echoing out a blank .ocamlformat (still echoes warning). This could fix contributing issues if a stray .ocamlformat is in a repository, but maintainer isn’t using it (either using another formatter, or prefers doing it by hand), then a contributor assumes they need to run ocamlformat (or their text editor is trying to format on their behalf) which causes unnecessary diffs + friction. (Personally, I use a Nix devShell per project to install formatters since each project tends do do something a little different).
  2. Echo build/ appropriate ignores for _darcs, .hg, .pijul. If .fossil is found, skip (doesn’t have local ignore AFAICT). If .git, .jj, or none of the aforementioned VCS folders are found, echo a .gitignore (default behavior). Hint-reminds darcs users to setpref as is convention.
  3. If any of the ignore files are found, append to them instead of overriding or skipping. This clears up some of the questioning in that open merge request and is required since pijul init creates a default .ignore.