Nightly builds are sometimes broken

[oliversalzburg]

I talked about this on Slack. The nightly build are sometimes broken.

I believe that the readFile call is invoked and then the gitutil.pendingChangesExist() call is made right after. Both calls have asynchronous behavior, which introduces a race condition. I believe that this race condition is to blame for the random build failures.

Looking around the file, there is at least another case of this pattern. There might be more in the module if this is a general oversight by the author.

My suggestion is, given the generator/yield approach of the module, the workflow should be adapted to that or, for a smaller refactoring, the code should use fs.readFileSync instead. In the latter case, refactoring the code should be trivial. I could create a PR for that if there is consensus about the change.

[raphinesse]

I find your analysis to be very plausible after a quick glance over the code. Given that there's already more sync stuff happening than async in this particular module (all the shelljs stuff is sync too), I would not object a fix that simply uses fs.readFileSync. An easy fix might be a good start anyway, so we can verify the fix in "production". But if you'd like to do a nice async version of this, I think nobody will be stopping you. Any PRs welcome!

I particularly like that someone already found the error but chose to ignore it 😅

cordova-coho/src/versionutil.js

Lines 92 to 93 in 5d710a3

	// TODO: why do we read files asynchronously in this function, but write
	// and check for existence synchronously?