Git commit throws error when no user.name/user.email is known in Git repo

hughbris / grav-plugin-pushy

Push with Git to publish changes to your production environment

MIT License

3 stars 1 forks source link

Git commit throws error when no user.name/user.email is known in Git repo #15

Open pamtbaau opened 2 years ago

pamtbaau commented 2 years ago

When committing pages to the Git repo (PushyRepo::commit()) , the Git library complains that user.name and user.email are unknown. They are however available at the global level.

Removing --author from execute command does not solve the issue
Adding a name <email> hardcoded in the commit statement does not help
shell: Executing commit without --author works fine.

A side effect of this is that changes have been staged and will no longer be visible in the list of pages in Admin. Manual unstaging is needed.

Suggestion:

When commit throws an exception, all staged items should be unstaged.

hughbris commented 2 years ago

Interesting. Yes there is great potential for git errors and they should be handled better. I don't know if it's best to wait for #11 though.

I thought I had coded the commit wrapper to use the logged in email/name as fallback, or perhaps I just thought it or added a TODO comment.

* When commit throws an exception, all staged items should be unstaged.

Agree, or is there some equivalent of database rollback available in PHP? That is, encapsulating statements into a "transaction".

pamtbaau commented 2 years ago

I thought I had coded the commit wrapper to use the logged in email/name as fallback

The username/email will have to be added to the config of the local Git first. Can the current lib do that?

Agree, or is there some equivalent of database rollback available in PHP? That is, encapsulating statements into a "transaction".

I'm afraid that as long as Git doesn't provide it, it would be impossible for any language to implement a language construct for it...

I'm also afraid that another library will not provide such action. It will probably remain a manual action inside the catch. Does the current lib provide a method to remove staged items?

hughbris commented 2 years ago

I'm not seeing the original problem reported here. My client made a commit through Pushy that was correctly attributed to her name and email, even though git is configured to use something else on that server.

I thought I had coded the commit wrapper to use the logged in email/name as fallback

Actually it uses it first with a TODO to "add a fallback".

The username/email will have to be added to the config of the local Git first. Can the current lib do that?

I don't know and the library needs replacing anyway. Perhaps I will add the author configured for the repo as a further setup requirement to the docs. I am unsure about the plugin going too far into repository metadata. I guess because I don't want it to be going into dangerous territory, or being perceived that way. It's kind of about building trust (I don't know).

hughbris commented 2 years ago

I'm also afraid that another library will not provide such action

That's probably right. I hoped one might emulate a rollback using some magic with PHP.

hughbris commented 2 years ago

I'm not seeing the original problem reported here.

Replying to myself, I thought just earlier that I really should be setting up a test site for working on this plugin so I can properly try to reproduce issues and generally work in safety. I have been working very carefully in a real repo which is sure to bite me one day :/