Skip to content
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

Remove top-level .gitignore #757

Closed
wants to merge 4 commits into from

Conversation

minhqdao
Copy link
Contributor

@minhqdao minhqdao commented Sep 19, 2022

Using the Modern Fortran plugin in VSCode, .mod files are continuously generated and I think it makes sense to exclude them from VC by adding them to .gitignore. The .mod files in the example folders are also excluded after the change.

Fixes #758. Modifications to the template are being done in another PR.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Given the discussion in #528 on generated .gitignore files we should actually remove the toplevel .gitignore from the fpm repository.

@minhqdao
Copy link
Contributor Author

minhqdao commented Sep 19, 2022

Given the discussion in #528 on generated .gitignore files we should actually remove the toplevel .gitignore from the fpm repository.

Yes, there is a duplication. But is it better to have a .gitignore (with * being its sole content) in the build folder rather than a top-level one? In other languages it is common to have a top-level .gitignore with the build folder (and many more) in it because there are, e.g., transient dependencies that need to get excluded as well.

@minhqdao minhqdao changed the title Ignore .mod files in version control Remove redundant .gitignore Sep 22, 2022
@minhqdao minhqdao changed the title Remove redundant .gitignore Remove top-level .gitignore Sep 22, 2022
@minhqdao
Copy link
Contributor Author

I removed the top-level .gitignore so it now parallels an fpm project created with fpm new. If it ever be decided to use a top-level .gitignore, I think we should change it in both places simultaneously.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me now.

@awvwgk awvwgk requested a review from a team September 22, 2022 12:22
@everythingfunctional
Copy link
Member

I'm actually of the opinion that a single, top-level .gitignore is better. It keeps all the related "configuration" information in one place.

@certik
Copy link
Member

certik commented Sep 22, 2022

What does fpm new do with regards to .gitignore? I don't see any .gitignore when I did fpm new, but maybe I have an older version.

@minhqdao
Copy link
Contributor Author

What does fpm new do with regards to .gitignore? I don't see any .gitignore when I did fpm new, but maybe I have an older version.

Ah, you're right. It's not fpm new but if I create a project with fpm new and then run it with fpm run, it creates a build folder with a .gitignore in it.

@certik
Copy link
Member

certik commented Sep 22, 2022

I see, it does for me as well. We might want to redesign this, and rather create a top level .gitignore in fpm. If you want, go ahead and submit a PR to fpm to do this.

@urbanjost
Copy link
Contributor

It originally was at the top level, but was moved to be in the subdirectory and always unconditionally created in the build directory when the directory is built/rebuilt to prevent problems where people were deleting or editing the .gitignore and ending up having the build directory tracked by git and copied into github repositories; and there was a possibility that fpm would allow you to change the name of the build directory to something like ".build" or a path to a writable area if say the files were owned by another user and you did not have write permission so you would not have to make a copy (adding that option did not happen so far) and so on.

So if you put back the top .gitignore I would also leave the file in the build directory.

Not everyone uses "fpm new" to create a project, and not everyone uses git(1). Someone changing an existing project to support fpm(1) may already have a .gitignore file, and so far "fpm new --backfill" does not change existing files. Since you know the build directory is "owned" by fpm you do not have to worry about collisions with existing files, etc.

@everythingfunctional
Copy link
Member

@urbanjost makes some good points. However, there is a point at which trying to "protect the user from themselves" has gone too far. I'm starting to think that maybe fpm should be version control system agnostic (eventually), and it should be the user's responsibility to create a .gitignore (or the equivalent for other VCS) if they want to. That's especially true if we add the feature to allow the user to specify the build folder location.

@awvwgk
Copy link
Member

awvwgk commented Sep 29, 2022

In case we allow fpm to select the build directory it seems especially useful to have a .gitignore in the build directory. While the main project developer can configure the .gitignore for their project, contributors choosing their own naming convention for the build directory might accidentally commit it again if it doesn't match any rule from the .gitignore.

As for other VCS repositories, like mercurial, there is .hgignore with similar syntax as for .gitignore which can also be present in any directory. However, we currently only support git projects with fpm, worrying about other VCS types can be deferred until support for them can be established with fpm (see #574 for example).

@everythingfunctional
Copy link
Member

especially useful to have a .gitignore in the build directory.

I kind of disagree. If you have a .gitignore file in the build folder, then when users delete the build folder (i.e. to start clean), they may inadvertently commit deleting it and then they're back in the same boat.

Dealing with version control and the systems/tools used for it is (IMO) outside the scope of fpm. We chose to use git for fetching dependencies because it was expedient at the time for proof of concept and getting people using it (i.e. we didn't have to invent our own methods for location and version specification and resolution), but the end goal is that fpm users won't have to use or touch git or any other VCS except as desired for the management of their own project.

Are there any other build or package management systems that interact with a VCS on behalf of their users? If so, what do they do?

@awvwgk
Copy link
Member

awvwgk commented Sep 29, 2022

If you have a .gitignore file in the build folder, then when users delete the build folder (i.e. to start clean), they may inadvertently commit deleting it and then they're back in the same boat.

How does this work? I'm not aware of any git commands (without --force adding and committing the build directory first) which would allow this. Having the .gitignore in the build directory should prevent exactly this scenario, unless a user overwrites the default behavior with --force.

.gitignore ignoring itself
❯ mkdir build
❯ echo '*' > build/.gitignore
❯ git add build/.gitignore
The following paths are ignored by one of your .gitignore files:
build/.gitignore
hint: Use -f if you really want to add them.
hint: Turn this message off by running
hint: "git config advice.addIgnoredFile false"

Are there any other build or package management systems that interact with a VCS on behalf of their users? If so, what do they do?

Meson always creates a .gitignore and .hgignore in any build directory (in-source build not allowed). Cargo creates a .gitignore file in the toplevel ignoring /target (the build directory, name is customizable via config / env. variable), similarly Stack creates a toplevel .gitignore for .stack-work/.

@everythingfunctional
Copy link
Member

How does this work? I'm not aware of any git commands (without --force adding and committing the build directory first) which would allow this. Having the .gitignore in the build directory should prevent exactly this scenario, unless a user overwrites the default behavior with --force.

I see. The intention is that file is created by fpm build on every invocation, and prevents also itself from being added. 👍

Meson always creates a .gitignore and .hgignore in any build directory (in-source build not allowed). Cargo creates a .gitignore file in the toplevel ignoring /target (the build directory, name is customizable via config / env. variable), similarly Stack creates a toplevel .gitignore for .stack-work/.

So other build systems do address this, but not all in the same way. If we're going to also, I guess I'm in favor of always creating it in the build folder.

@urbanjost
Copy link
Contributor

Creating build/.gitignore is part of creating the build/ directory, so even if you delete it it creates a new .gitignore in the directory to prevent some of the problems described here with deleting build/ and deleting or not having a .gitignore in the project root. I have projects that do not use git(1) for RCS. I just ignore git except when used to access remote directories.
A little .gitignore file in an fpm-built directory is helpful even then, as the revision control systems do not interfere with each other (or at least git does not, which is enough) so I can run a little script to put files under git as well and push it to github,
for example. You can build a relatively complex project with links to other project roots instead of using git clone dependencies as well. I use a simple convention of pulling projects from github to $HOME/github and then making links in the project under a directory called LINKS so it is easy to work off-line and to reduce duplication among other things, as an example. A few things like globbing program names, the include/ directory, and using links instead of git clones needs some more examples in the documentation.

@eli-schwartz
Copy link

Creating VCS ignore files as part of generating a build directory (which is what Meson does for both VCSes it knows about) is completely free and doesn't stop people from also creating a top-level gitignore.

But it does mean that users get the benefit of it even if they forgot or never thought about doing it.

...

Once you're already having the build directory automatically ignore itself, why not take advantage by simplifying or potentially even getting rid of the gitignore checked into git? If the tool is smart enough to handle this for you, then you get to put in the work once, in one tool, and benefit again and again. Users then don't know anything other than "gosh, this tool is so awesome and clever, because somehow magically when I use it I never accidentally commit build artifacts to git".

@urbanjost
Copy link
Contributor

I believe that other changes have resulted in this being resolved, but not sure. Current releases of fpm create a .gitignore in the build directory if it is not present each time a build is generated; and generate no top-level .gitignore file. Is that the intent here (in which case I think this can be closed) or does VSCode need a top-level .gitignore because it generates scratch files in other locations? If so, it seems that should be addressed by VSCode (??).

@minhqdao
Copy link
Contributor Author

minhqdao commented Dec 30, 2022

The intent here was to remove the top-level .gitignore in the fpm repo, not what is created by fpm new. Because build folders have their own .gitignore (ignoring everything that's inside), a build/* in the top-level .gitignore should be redundant.

That top-level .gitignore files aren't created by fpm new has been part of this PR: #528

But you're correct with the .vscode folder. I think it's less about scratch files, but devs often put a local, project-specific settings file in there that should not be committed. It's useful to have it in the top-level .gitignore and has been added recently.

Maybe I should just remove the build/* from the top-level .gitignore then.

@minhqdao minhqdao requested a review from awvwgk December 30, 2022 13:56
@eli-schwartz
Copy link

Personally I think .vscode belongs in the user's own global per-user gitignore rather than checking it into git for every project on all of GitHub. :P

Adding it to project gitignores means that many projects get churn, and then vscode users still have issues for any projects that don't do that.

@minhqdao
Copy link
Contributor Author

Both options are fine for me. 🙂

@minhqdao
Copy link
Contributor Author

Maybe we can resolve it this way: @gnikit, you recently added the .vscode folder to .gitignore, how do you feel about putting it in the user's own global .gitignore instead and have the top-level .gitignore removed entirely?

@minhqdao minhqdao closed this Jun 2, 2023
@minhqdao minhqdao deleted the ignore-mod-files branch June 2, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Put .mod files on .gitignore
6 participants