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

fix: fix ENAMETOOLONG for too big commit messages #263

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ChristianUlbrich
Copy link

This PR allows running semantic-release to reliably commit arbitrary big commit messages. The old way of adding a commit message was by passing it simply along as a command line argument to git commit -m ${message} this will easily break for huge commit messages, depending on the platform:

  • Windows seems to limit the total length of a program execution "string" (and thus a execa() call to roughly 8 KB
  • and there are limits for the total length per passed argument (on my system 128KB)

In practice one can easily hit those limits by having a moderate number of changes during releases, because upstream in semantic-release for a default setup of semantic-release/changelog, semantic-release/git the generated changelog is added as a commit message.

The fix is, that the message is not passed as an argument, but a temporary file containing the message will be created and the file will be passed instead.

There are now some additional implications; whenever something goes wrong, there is the possibility that temporary files are not deleted, however as os.tmpdir() is used, this is something that the OS should deal with. Furthermore this now will break, if os.tmpdir() is not writeable by semantic-release. However I think it is a fair assumption, that os.tmpdir()is writeable - although an additional check could be added.

Whether this is a breaking change, this is left as an exercise for the astute reader something to be answerer by the maintainers. I'd say it is not, because it is behaving as intended. However this might break some pipelines, that have not os.tmpdir() writeable. I tested it on an Azure DevOps Windows hosted agent and I am pretty sure™ that in a sane default™ setup of build agents this should not pose any problems.

This is somehow related to: #91 (although I think the actual course for the problem there was the amout of changed files); at least it is giving the same error message.

There are further places in the code, where seemingly unrestricted data is passed via execa() that is suffering from the same problems (adding a lot of files via add), however fixing this is not so easy, because git add does not support adding files from a "list file". Possible solutions would be a chunked adding (splitting the files by "length" and thus multiple git add calls) - however this has further implications. I'd rather fix those problems in another PR if the maintainers are fine with this approach (or suggesting sth. better).

@ChristianUlbrich
Copy link
Author

@pvdlg Ping, is there anything not clear? This is breaking our build currently and while using the fix directly is really easy (because all plugins are old-school w/o any bundling yay!), our team lead is somewhat hesitant using private fixes from GitHub. :)

@ChristianUlbrich
Copy link
Author

@travi BUMP. This is still a problem... Luckily the one year old fix, still happily merges. :)

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.

None yet

1 participant