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

Fix: add wait when linking library with *.resp file #808

Merged
merged 4 commits into from
Feb 22, 2023

Conversation

zoziha
Copy link
Contributor

@zoziha zoziha commented Nov 28, 2022

Description

Fix #807 .

Linking the link library takes a while, the *.resp file is occupied, if fpm delete the resp file without waiting for linking to complete, we will get an error, this PR intends to fix it.

  • Remove OMP critical;
  • Add wait for linking.
  • Use local delete_file_win32.

@zoziha zoziha requested review from LKedward and a team November 28, 2022 15:10
Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@LKedward
Copy link
Member

Thanks @zoziha, however I'm a little confused. I thought the default for execute_command_line was to be synchronous if wait isn't present, so adding wait=.true. shouldn't change the existing behaviour should it? Or have a misunderstood?

@zoziha
Copy link
Contributor Author

zoziha commented Nov 30, 2022

@LKedward , I agree with your point of view, I think this PR has no substantial progress, it is recommended to close. Sorry that I was wrong.

The cause of Fortran runtime error: File cannot be deleted is still unknown at present, this error still occurs at a very very low frequency, should we consider adding iostat for the close in delete_file? Turn it into a warning.
(It might even be possible to ignore this error since it's not actually a compilation error, or add extra way to delete the resp file?)

@LKedward
Copy link
Member

No need to apologise! I appreciate you looking into this issue @zoziha , I have also noticed it still cropping up sporadically (though less frequently than before).

... should we consider adding iostat for the close in delete_file? Turn it into a warning.

This seems like it would be a good idea! We just need to be careful that other parts of fpm don't rely on delete_file always working, though I don't think that is the case.

Another option could be to use iostat to retry the delete operation 1 or 2 times - I suggest this since I suspect the underlying issue could be with the Windows OS and not with anything in our code.

@zoziha
Copy link
Contributor Author

zoziha commented Nov 30, 2022

Thanks for the reply @LKedward , I think this is a known problem on Windows, we should probably do additional processing based on the iostat value of close: ignore it (fpm will try to delete resp file again the next time it builds the code), continue to try to delete, or change the deletion method, etc. .
We can choose a method we think is appropriate to deal with it.

@awvwgk , do you have any good suggestions for this?

@zoziha
Copy link
Contributor Author

zoziha commented Dec 5, 2022

I returns iostat in the delete_file, but do not process iostat, so the consequence is that resp has a probability not been removed. It may be replaced in a later fpm build.

I tried to delete the resp file one more time according to iostat, but the effect was that resp was still not deleted. This is what I can be sure of.

The extra way is to add a subroutine that calls the shell's rm program to remove the resp file, but it may not be very necessary.

@awvwgk
Copy link
Member

awvwgk commented Feb 4, 2023

Is this patch ready to go?

@zoziha
Copy link
Contributor Author

zoziha commented Feb 12, 2023

Since #807 only happens on Windows OS, I think it might be a good decision to use preprocessing to declare a unique delete_file_win32 only for Windows.

By the way, I think this PR has nothing more to submit, it is ready to go.

src/fpm_compiler.F90 Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

fpm fails to compile projects in parallel: Fortran runtime error: File cannot be deleted (2)
4 participants