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

composer.lock does not match vendor directory #322

Closed
axelvnk opened this issue Feb 12, 2018 · 16 comments
Closed

composer.lock does not match vendor directory #322

axelvnk opened this issue Feb 12, 2018 · 16 comments

Comments

@axelvnk
Copy link

axelvnk commented Feb 12, 2018

example :
elasticsearch/elasticsearch

  • composer.lock : version 2.1.5
  • actual version in vendor dir : 2.3.0

Me as a developer just executes composer install when I see a lock file, not really noticed the vendor directory. Had me looking for hours why I had certain errors.

We have to fix the composer.lock file. But I would also really like to completely remove the vendor directory. But I guess the reason why it's here in the first place is for sitebuilders. We can also just add a composer.phar and run php composer.phar install from the install script.

I'll open up a PR for the lock file.

@inquisite
Copy link

We include vendor in Git and distributions because 95% of our users don't know about or want to know about Composer. You can delete the vendor dir and lock file and run it yourself of course.

@axelvnk
Copy link
Author

axelvnk commented Feb 12, 2018

Yes, I just acknowledged that... But the version in composer.lock does not match the actual version that is included. Removing composer.lock and running composer install does solve the problem, but why would you have a composer.lock file in there anyway if you don't care about the actual version installed?

@inquisite
Copy link

Good point. We'll get rid of it.

@axelvnk
Copy link
Author

axelvnk commented Feb 12, 2018

:D not what I meant. You SHOULD have it if you really want to have the vendor directory in there, but it MUST match the packages versions in the vendor directory. Just in case someone like me executes a composer install, that way the exact same versions that you're counting on are installed and used. And doesn't end up breaking stuff.

@kehh
Copy link
Contributor

kehh commented Feb 12, 2018

We got bitten by this the other day - I thought there was some crazy dependency hell that was resulting in the downgrade to ElasticSearch. Thanks @axelvnk for picking this up.

My personal inclination for minimal confusion:

  • If people are installing from git, then they should be prepared / instructed to use composer and the vendor directory should be excluded from git.
  • If people are downloading the package from a release then this should include the vendor directory.
  • Having the vendor directory in VCS can result in time consuming merge conflicts of code that we're not familiar with.

Obviously other opinions are available and I'm certainly a fan of pragmatism over pedantry.

@inquisite
Copy link

Yeah, i know it needs to be there. I mean the incorrect one needs to go.

Kehan: I fully agree that having vendor/ in Git is ugly and annoying and just plain stupid. So long as you don't run composer on your install you won't have conflicts, but of course that's not really a solution. The issue for us is that a whole lot of people are running off of GitHub and are not inclined to, or capable of, dealing with composer. Our response to the resultant support headaches was simply to bring in copies of dependencies, first in app/lib/ and later in vendor/. This creates support headaches from people like you, but you are fewer in number :-)

In the short term this will have to remain as it is. In the medium term we will get rid of vendor/ and require composer use. We can hopefully mitigate some of the support issues with a simple script to do the composer install and initial run. This will dovetail with removal of most assets/ and replacement with NPM. Work on that is supposed to start this week.

@kehh
Copy link
Contributor

kehh commented Feb 12, 2018

Ooohhh oooohhhh ohhhh. Exciting!

@axelvnk
Copy link
Author

axelvnk commented Feb 12, 2018

As I suggested before, I think we can just include a composer.phar file and use it to execute a composer install from a PHP file. Like during setup.php or something.. That way the sitebuilders have no clue that some composer stuff is going on and we can still have our sweet sweet versioning 👍

@inquisite
Copy link

That would be nice, but quite a few of our users don't actually have command-line access to the system they install on. Maybe it can be triggered from setup.php. We'll see if that can work.

@collectiveaccess
Copy link
Owner

The dev/metadataAlerts branch now has functioning auto-pull-from-composer mechanism. This branch is the develop branch with a couple of major changes beyond composer:

• Includes a "metadata alerts" feature that can send notifications triggered by modifications or dates/times
• Simplified setup.php format

http://clangers.collectiveaccess.org/jira/browse/PROV-2407

@jtreminio
Copy link

A better solution to including /vendor in your repo is providing a packaged archive for end users that includes everything they need to get up and running with minimal interaction.

@collectiveaccess
Copy link
Owner

Master now includes /vendor as it's a release. All other branches pull from composer.

@dusta
Copy link

dusta commented Dec 7, 2020

@collectiveaccess This is very incorrect. Vendor should be removed from repo. There is many websites that allow you to download ready code witout composer. For example https://php-download.com/

@collectiveaccess
Copy link
Owner

Vendor is only included for releases.

@dusta
Copy link

dusta commented Dec 7, 2020

@collectiveaccess But problem is that repo it gets huge by this. As I know you can include separate package in to release with builded application.
obraz

@collectiveaccess
Copy link
Owner

Yeah good point. We'll look into alternatives.

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

6 participants