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

Why is composer.lock included in project? #21

Closed
lautreamont3 opened this issue Jan 9, 2014 · 17 comments
Closed

Why is composer.lock included in project? #21

lautreamont3 opened this issue Jan 9, 2014 · 17 comments

Comments

@lautreamont3
Copy link

Meybe I'm wrong, but this way I always need to run composer update after instalation.

@swalkinshaw
Copy link
Member

We could (and maybe should) leave it out of this repo because we aren't using any wildcard dependency versions so they don't need to be "locked" down. The minor issue is we don't want to add it to .gitignore so every contributor would have to be careful to never commit it.

@swalkinshaw
Copy link
Member

Going to close this for now due to the reason above. May re-visit it later.

@benjibee
Copy link
Contributor

benjibee commented Feb 6, 2014

I'd like to second this. The first step of my Bedrock setup was adding the lock file to the .gitignore as is done with all of my projects.

@swalkinshaw
Copy link
Member

@benjibee your composer.lock file should always be in version control for actual project repos. See https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file

@benjibee
Copy link
Contributor

benjibee commented Feb 6, 2014

@swalkinshaw yes, I understand this when the given dependency version is * or dev-master for example. I tend to use exact versions in my composer.json for stability as does this project, so I leave the lock file out of the repo.

Anyway, it's a minor thing, I just wanted to give my 2¢, sorry to "bump" a closed issue!

@QWp6t
Copy link
Member

QWp6t commented Feb 6, 2014

👎

@swalkinshaw
Copy link
Member

@QWp6t dislike removing the file or dislike that we're keeping it in Bedrock?

@QWp6t
Copy link
Member

QWp6t commented Feb 8, 2014

I don't agree with removing the lock file. I agree with you that the lock serves an important function.

The idea behind the lock file is that you can include the lock file instead of including dependencies.

If you're not including the dependencies in the repo, then the lock should be included; this is especially important if the development repo doubles as your distribution channel (as is the case with bedrock).

Even if your dependencies aren't wildcards, the lock is still a good idea because it is more specific.

@bitwombat
Copy link

Necrobump, sorry.

The lock provides a useful function, but committing it to the project defeats its purpose.

The .lock file's purpose is to lock my clone of the repo, after I run composer, to particular versions of dependencies. This lets me fetch the latest-and-greats versions, within the limits specified in the project's composer.json file.

If the exact version matters, it can be specified that way in composer.json. The .lock file is supposed to be created by my run of composer.

I hope this makes sense and doesn't seem contentious.

Not a huge deal, just not "proper", I don't believe, and something that jumped out at me on first look.

@kalenjohnson
Copy link
Contributor

bitwombat, I'm not following your reasoning. You're talking like once a lock file is included in the repo, the dependencies can never be changed, which is obviously not true.

If you want a set dependency, or a new one, modify the composer.json file, run composer update, and the lock file will be updated, and it will be all yours. So where is the issue you're experiencing?

@austinpray
Copy link
Contributor

FWIW bedrock's lockfile is updated by an automated script every time WP updates

@bitwombat
Copy link

I retract this and stand corrected. Composer's own docs say to commit the lock file (as @swalkinshaw said above). Though not for libraries - perhaps why I've rarely seen this.

One reasonably rated answer on Stack Exchange makes the case I did above, "but still", I now think it's correct as is.

Basically, committing the lock file is saying to your users "Use these, they're what we tested with", whereas wildcards in composer.json are saying, to devs, "We're willing to test with any of these".

@roots roots locked and limited conversation to collaborators Jan 27, 2016
@roots roots unlocked this conversation Sep 27, 2019
@aaemnnosttv
Copy link
Contributor

I think there may be a bit of misunderstanding here. The issue is only about whether or not the composer.lock file should be included in this repo - not about whether or not projects based on Bedrock should use/include a composer.lock file in their repos.

I think everyone agrees that including the lock file with your app/project is the way to go. I don't think this repo should add composer.lock to its .gitignore, but I can see the reasoning behind omitting the lock from this repo.

The lock file will be created on the first install or create-project, so the end result would still "include" the file either way in the fresh codebase. The difference is a smaller footprint in this repo, and potentially a better initial setup experience as the latest versions according to the defined constraints would be installed by default.

With this in mind I think "Why is composer.lock included in project?" is a valid question.

At the end of the day, I don't think it really matters all that much but maybe there's a better reason to go one way or the other.

Does dependabot require the lock file to work? Is dependabot even necessary without it?

Apart from potential integrations/inspectability I don't see a strong reason to include the lock file in this repo, but I would be interested to know if there are others.

@austinpray austinpray reopened this Sep 27, 2019
@devkinetic
Copy link
Contributor

My take on this is that when using Bedrock as an upstream in git, I get conflicts on the lock file upon merging. I handle this by keeping my local version, and then updating lock after the merge is complete, which is a necessary step anyways. It would be nice to not have this merge issue, but it's a pretty common thing within our CI across multiple composer based projects, and the way to deal with it is always the same. It speak more to the composer file having stable constraints, and proper testing before it makes it way to a release.

@jordanlgraham
Copy link

I'm in favor of leaving composer.lock in this repo, as it gives critical information on which versions of dependencies work with the project's code.

For example, in a Bedrock-based project we removed the vendor directory and composer.lock and installed, which pulled vlucas/phpdotenv version 4.1.0, in which dotenv's create() method no longer accepts a string as its argument.

By reviewing this repo's composer.lock file we were able to see on which version of dotenv the application.php file is based, allowing us to alter our code to be compatible with version dotenv v4.1.0.

@aaemnnosttv
Copy link
Contributor

I'm in favor of leaving composer.lock in this repo, as it gives critical information on which versions of dependencies work with the project's code.

This is what the composer.json's require blocks define 😄

For example, in a Bedrock-based project we removed the vendor directory and composer.lock and installed, which pulled vlucas/phpdotenv version 4.1.0, in which dotenv's create() method no longer accepts a string as its argument.

What you're describing is a problem regarding the version constraint you have in your composer.json. It sounds like you've modified yours as this repo has never used a constraint which could select v4 of vlucas/phpdotenv. Currently this is ^3.6.0 which will install the latest release from 3.6.0 up to but not including 4.0 or later.

@swalkinshaw
Copy link
Member

swalkinshaw commented May 29, 2021

There's been some good discussion in here over 7 years (😱 ) but I'm closing this since it's unlikely we'll change this after so long.

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

No branches or pull requests

10 participants