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

Can not fix file via --update #62

Closed
voodoo66 opened this issue Sep 20, 2020 · 16 comments
Closed

Can not fix file via --update #62

voodoo66 opened this issue Sep 20, 2020 · 16 comments

Comments

@voodoo66
Copy link

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

Call arguments:
win_bison --update file.y

Output:
cannot backup: Permission denied

@lexxmark
Copy link
Owner

will look into it

@lexxmark
Copy link
Owner

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

@voodoo66
Copy link
Author

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

@lexxmark
Copy link
Owner

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
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Owner

lexxmark commented Oct 23, 2020

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

@GitMensch
Copy link
Collaborator

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?

  • investigate if the gnulib module is used, and if not if it would solve the issue (using the fopen module may be easily done upstream)
  • work around the issue by manually wrapping fopen as mentioned in the article - in this case likely not replacing the calls in the code but add an extra definition and a #define fopen fopen_unixlike

@GitMensch
Copy link
Collaborator

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Collaborator

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

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

No branches or pull requests

3 participants