lexxmark / winflexbison

Main winflexbision repository
GNU General Public License v3.0
412 stars 119 forks source link

Can not fix file via --update #62

Closed voodoo66 closed 4 years ago

voodoo66 commented 4 years ago

Seems renaming file fails before the update process can be executed.

Call arguments: win_bison --update file.y

Output: cannot backup: Permission denied

lexxmark commented 4 years ago

will look into it

lexxmark commented 4 years ago

I invoked win_bison.exe --update calc.y without any errors on the latest version Please provide more details to reproduce

voodoo66 commented 4 years ago

If you introduce some updatable code it will fail, e.g.: %error-verbose

lexxmark commented 4 years ago

Thank you, I have reproduced it.

The input file remain opened (by caret_info global variable) during renaming calc.y to calc.y~ in fixits_run() function.

It seems on Linux it's legitimate operation. Please correct me if I'm wrong. On windows this situation leads to a Permission denied error.

There are two options:

  1. Report a defect to upstream bison project with hope it will be fixed there (it seems not really a problem on linux)
  2. Try to fix it in winflexbison (may be send pull request to bison project afterwards)
GitMensch commented 4 years ago

You can rename files with an open handle normally fine (all open handles will still be valid), on linux you can additionally delete or overwrite the file (all open handles will still be valid, on Windows the delete/overwrite fails).

But concerning the current issue: discussing with upstream seems reasonable, then likely close, copy, reopen.

lexxmark commented 4 years ago

You can rename files with an open handle normally fine (all open handles will still be valid)

Are you sure it's true for windows? Why we are getting Permission denied error?

GitMensch commented 4 years ago

I'm sure that this is true when you rename a file via Windows Explorer which may means that either different ways of doing the rename are needed or that only a different process can rename the file.

lexxmark commented 4 years ago

I was performing debugging and after opening file in win_bison tried to rename it in Windows Explorer. It didn't allow me with error that file is used in win_bison

image

GitMensch commented 4 years ago

Hm, can you rename the containing folder? if not then my mind has played me a trick and this is only possible for files opened for read (which does work, doesn't it?)

lexxmark commented 4 years ago

I found this article http://blog.httrack.com/blog/2013/10/05/creating-deletable-and-movable-files-on-windows So windows fopen function locks file in any mode ('r' as well).

lexxmark commented 4 years ago

In bison code I found two "fopen" calls. It makes sense to replace them with function from article.

GitMensch commented 4 years ago

Thank you for sharing the article. I've had a hope that the gnulib fopen module (not checked if bison uses it already) would fix this, but as https://www.gnu.org/software/gnulib/manual/html_node/fopen.html doesn't mention that part i guess not.

So what should be done next?

GitMensch commented 4 years ago

Rechecked: the current gnulib fopen imnplementation does nothing like the article, it may still be most reasonable to adjust this module, use it and send it upstream to gnulib. Then let upstream bison use the updated module (preventing the need for patching again and again).

What do you think?

Side note: I'm just remembering that we have #17, using those would likely have caught this issue up-front...

lexxmark commented 4 years ago

I haven't found fopen definition in the winflexbison code, it uses system function defined in "C:\Program Files (x86)\Windows Kits\10\Include\10.0.18362.0\ucrt\stdio.h"

I replaced fopen with fopen_unixlike and it solved the problem. Will use define you suggested #define fopen fopen_unixlike

lexxmark commented 4 years ago

Side note: I'm just remembering that we have #17, using those would likely have caught this issue up-front...

It seems I fixed the problem with binary mode. If you need something else from me there let me know.

GitMensch commented 4 years ago

When you already define the function I'd suggest to add that part of the gnulib definition "just in case":

#if defined _WIN32 && ! defined __CYGWIN__
  if (strcmp (filename, "/dev/null") == 0)
    filename = "NUL";
#endif