-
Notifications
You must be signed in to change notification settings - Fork 232
Update last modified date directly in touch
#2014
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
Update last modified date directly in touch
#2014
Conversation
This mitigates race conditions in which concurrent Pub processes "touch" pubspec.lock around the same time and leave it with a trailing NUL character.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! (My company, Workiva, signed it!) I know in the past, our CLA hasn't been picked up by googlebot. I see that the CLA instructions have changed, so perhaps I need to get in touch with our Point of Contact. I don't know who that is, though |
cc @mit-mit |
What do you see if you go to https://cla.developers.google.com/clas ? Did the git commit use an @workiva.com email address? |
I see this: Yes, the commit uses |
Got it, let me do some debugging on my end |
Thanks!! |
Can you verify that you are a member of [email protected]? That's the group that the class is tied to. |
Hm, not that I can tell. Let me ask around... |
Hey @mit-mit, I just got added to that group! |
CLAs look good, thanks! |
@jonasfj & @natebosch can you take a look at this PR? |
🎉 Any idea which Dart SDK version this will be released in? |
Need to merge it into the SDK first. Then it'll be "the next one". Updated dart-lang/sdk#36026 to make the request for @sigurdm |
This mitigates race conditions in which concurrent Pub processes "touch"
pubspec.lock at the same time and leave it with a trailing NUL character.
Long-winded background info
We've been seeing intermittent failures in our automated builds for some time now, which looked like this:
However, we had a hard time tracking them down. Eventually, we determined that the unexpected character in that message was actually a NUL character.
"What was modifying
pubspec.lock
and adding a NUL character?" We weren't in our scripts, and there shouldn't be any reason for Pub to modify it since in all cases we've seen this issue, the pubspec.lock is checked in and didn't change afterpub get
.However, looking at Pub's source, it looks like its
touch
utility operates by writing a NUL character to the end of the file and then writing to it again with the original contents.We were running multiple Pub commands in parallel, which would explain this issue: two Pub processes could be performing
touch
at the same time,resulting in a NUL character being preserved in the output.
pub run
also always touchespubspec.lock
when there are path dependencies, which we had in all the repos we encountered this issue in, which explains why we were seeing it so often.To reproduce locally: with the following files...
main() {}
... run the following command in several of terminals at once;
eventually you'll get "Unexpected character" errors.