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

Pre-commit hook to regenerate composer lock hash? #3

Open
balbuf opened this issue Mar 28, 2018 · 5 comments
Open

Pre-commit hook to regenerate composer lock hash? #3

balbuf opened this issue Mar 28, 2018 · 5 comments
Labels
enhancement New feature or request

Comments

@balbuf
Copy link
Owner

balbuf commented Mar 28, 2018

Check for our special message and regenerate the hash as necessary - this would be an optional hook to add, of course.

@balbuf balbuf added the enhancement New feature or request label Sep 12, 2018
@eelkeblok
Copy link

Just out of curiosity, why can't the hash be generated when performing the merge?

@balbuf
Copy link
Owner Author

balbuf commented Feb 23, 2019

@eelkeblok there are a few issues that make it problematic:

  1. depending on how many dependencies there are, composer update --lock can take a really long time (even though it isn't updating anything); in general, this would be acceptable only if the user is ok with it (i.e. it would be an optional setting to enable)
  2. inside of the merge handler, we have no insight into any other files that were or will be merged, so, when we are handling one of the files (lock or json), we don't know if we've already handled the other file, whether it had any merge conflicts, etc. We need all of that resolved before regenerating the hash, otherwise we'd have to run composer update --lock after handling each file (i.e. possibly twice during the merge, and running it once already takes a long time).
  3. If there is a merge conflict in either file, we have to wait for the user's manual intervention before we can generate a proper hash, so it'd have to be done outside of the automatic merge in that case anyway. (And, we'd possibly be left with an incorrect hash if, for instance, the json file had a conflict but the lock file didn't. In this case we wouldn't have a notice that the hash is, in fact, incorrect.)
  4. because regenerating the hash involves two files (i.e. reading from composer.json and writing to composer.lock), I'd consider it outside of the scope of a merge handler, which is only supposed to handle the merging of different versions of a single file.

So really it only makes sense to do this right before a commit is made so that all the dust settles (both files are merged and conflicts have been resolved). This would ensure it is run consistently (after a merge conflict or not) and only once per merge.

@eelkeblok
Copy link

Interesting. I would've expected the content hash is only derived from the contents of the composer.lock, so that with the result of the merge you'd have all information needed to regenerate (provided there are no conflicts based on the enhanced merging this project enables). That does make this quite a bit less useful than I thought; the hash is often is the only merge conflict based on regular text-based merging.

@balbuf
Copy link
Owner Author

balbuf commented Feb 25, 2019

You're right, if that is the primary merge conflict you face. However, that is also the easiest conflict to fix. Simply remove the entirety of the conflict markers and conflicting lines pertaining to the content hash, save the lock file, then run composer update --lock.

@eelkeblok
Copy link

Well, yes, but exactly because it’s so easy I was looking for a way to automate it. We use a pull request workflow which will obviously fail to merge something when there is a merge conflict and with various branches adding different composer packages, we are guaranteed to be “solving” various such conflicts a day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants