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

re2c output is not atomic #245

Closed
terpstra opened this issue Mar 1, 2019 · 5 comments

Comments

@terpstra
Copy link

commented Mar 1, 2019

When I run this command and then interrupt it:
re2c -8 --no-generation-date src/symbol.re -o src/symbol.cpp

It will create an empty symbol.cpp file. This is problematic for make because the output now exists and so the job does not get rerun. To work around it, I must do this:

%.cpp: %.re
re2c -8 --no-generation-date $< > $@.tmp
mv $@.tmp $@

However, it would be nicer if '-o' did this internally.

@skvadrik

This comment has been minimized.

Copy link
Owner

commented Mar 4, 2019

Yes, this seems useful. The question is, how to create a temporary file portably in C++98?

One possible way is to open filename.tmp in the output directory (the name can be made a bit more random, e.g. to include timestamp) and fail if the file already exists. For that I think we can use open with O_EXCL on POSIX systems (which should cover Unixes, BSDs and macOS) and _open with _O_CREAT|_O_EXCL on Windows.

Out of curiosity, is your output file large? It seems that in most cases re2c does not take that long to generate output.

@terpstra

This comment has been minimized.

Copy link
Author

commented Mar 4, 2019

My re2c output is 944kB. More relevant, however, is it takes 13 seconds to run... and re2c seems to open the file fairly early in the process.

@terpstra

This comment has been minimized.

Copy link
Author

commented Mar 4, 2019

FYI, we will be open sourcing our tool which used re2c (wake) in the next few months, so you can see what all my complaining was driving towards. ;)

@skvadrik

This comment has been minimized.

Copy link
Owner

commented Mar 5, 2019

That's really good --- 13s sounds way too long, and I hope it can be reduced.

@skvadrik

This comment has been minimized.

Copy link
Owner

commented Mar 5, 2019

Pushed a fix: 2d30bc1, let's see how it works. Thanks to tmpfileplus for inspiration.

@skvadrik skvadrik closed this Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.