[Bug fortran/62215] New: Updating .mod files on Win32 fails

[Bug fortran/62215] New: Updating .mod files on Win32 fails

[jeffrey.armstrong at approximatrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

            Bug ID: 62215
           Summary: Updating .mod files on Win32 fails
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jeffrey.armstrong at approximatrix dot com

Created attachment 33373
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33373&action=edit
Patch to delete existing .mod file prior to updating

Under Win32 using GNU Fortran 4.9.0 or 4.9.1, module files (.mod) are not
updated properly when necessary because the code to do so is simply calling
"rename()" to move the temporary .mod file to the permanent name without first
deleting any existing .mod files.  The Windows C runtime states that this
operation will fail if a file of the same target name exists already (see the
"Remarks" section at http://msdn.microsoft.com/en-us/library/zw5t957f.aspx).

Versions 4.8 and earlier of GNU Fortran first performed an "unlink()" on the
existing file to delete it, so this bug is new with the 4.9 series.

I've attached a patch for 4.9.1 that reinstates the unlink operation that was
present in 4.8 and earlier.

[Bug fortran/62215] Updating .mod files on Win32 fails

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jb at gcc dot gnu.org,
                   |                            |kargl at gcc dot gnu.org

--- Comment #1 from kargl at gcc dot gnu.org ---
(In reply to Jeffrey Armstrong from comment #0)
> Created attachment 33373 [details]
> Patch to delete existing .mod file prior to updating
> 
> Under Win32 using GNU Fortran 4.9.0 or 4.9.1, module files (.mod) are not
> updated properly when necessary because the code to do so is simply calling
> "rename()" to move the temporary .mod file to the permanent name without
> first deleting any existing .mod files.  The Windows C runtime states that
> this operation will fail if a file of the same target name exists already
> (see the "Remarks" section at
> http://msdn.microsoft.com/en-us/library/zw5t957f.aspx).
> 
> Versions 4.8 and earlier of GNU Fortran first performed an "unlink()" on the
> existing file to delete it, so this bug is new with the 4.9 series.
> 
> I've attached a patch for 4.9.1 that reinstates the unlink operation that
> was present in 4.8 and earlier.

Looks like Janne removed that chunk of code in r198023.
Janne, is there any reason that this cannot be restored?

Jeff,
Do you have a copyright assignment on file?  Although the
patch is small and reverts to older code (which means an
assignment is most likely not necessary), if you plan to 
contribute in the future, then having the assignment on
file would be nice.

[Bug fortran/62215] Updating .mod files on Win32 fails

[jeffrey.armstrong at approximatrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #2 from Jeffrey Armstrong <jeffrey.armstrong at approximatrix dot com> ---
I do not have a copyright assignment on file, but this patch is pretty minor. 
I'm willing to fill one out (after I read it, of course), but it shouldn't hold
up this patch.  The new lines in the patch are taken verbatim from 4.8.2.

[Bug fortran/62215] Updating .mod files on Win32 fails

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #3 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Thu, Aug 21, 2014 at 05:21:50PM +0000, jeffrey.armstrong at approximatrix
dot com wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215
> 
> --- Comment #2 from Jeffrey Armstrong <jeffrey.armstrong at approximatrix dot com> ---
> I do not have a copyright assignment on file, but this patch is pretty minor. 
> I'm willing to fill one out (after I read it, of course), but it shouldn't hold
> up this patch.  The new lines in the patch are taken verbatim from 4.8.2.
> 

Not having a copyright assignment won't interfere with this
patch being applied.  I'm waiting for confirmation from Janne
that I can revert the changes in his previous patch.

The reason I asked about an assignment is that I am aware of
SimplyFortran (which looks like a nice product, but I'm not a
Windows user so haven't tried it).  Every once in a while, 
I'll scan the forums for gfortran issues.  I'm hoping that
you may consider helping to fix other issues with gfortran.

[Bug fortran/62215] Updating .mod files on Win32 fails

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Comment on attachment 33373
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33373
Patch to delete existing .mod file prior to updating


>       /* Module file have changed, replace the old one.  */
>+      if (unlink (filename) && errno != ENOENT)
>+	gfc_fatal_error ("Can't delete module file '%s': %s", filename,
>+			 xstrerror (errno));
>+             
>       if (rename (filename_tmp, filename))
> 	gfc_fatal_error ("Can't rename module file '%s' to '%s': %s",
> 			 filename_tmp, filename, xstrerror (errno));

POSIX states:
"If  the link named by the new argument exists and the file's link count
becomes 0 when it is removed and no process has the file open, the space
occupied by the file shall be freed and the file shall no longer be accessible.
If one or more processes have the file open when the last link is removed, the
link shall be removed before rename()  returns,  but  the removal of the file
contents shall be postponed until all references to the file are closed."

Thus, I wonder whether one should use an #ifdef instead of simply reinstating
the previous version. For instance, "#ifdef __MINGW32__". (I don't know what
__CYGWIN__ does, nor whether any other platform has the same issues.)
>From gcc-bugs-return-458991-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Thu Aug 21 19:30:58 2014
Return-Path: <gcc-bugs-return-458991-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 25309 invoked by alias); 21 Aug 2014 19:30:57 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 24989 invoked by uid 48); 21 Aug 2014 19:30:51 -0000
From: "tkoenig at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug fortran/62076] [4.9/5 Regression] testsuite failure in udr2.90
Date: Thu, 21 Aug 2014 19:30:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: fortran
X-Bugzilla-Version: unknown
X-Bugzilla-Keywords: openmp
X-Bugzilla-Severity: normal
X-Bugzilla-Who: tkoenig at gcc dot gnu.org
X-Bugzilla-Status: RESOLVED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 5.0
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: bug_status resolution
Message-ID: <bug-62076-4-kx3ka7TEgy@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-62076-4@http.gcc.gnu.org/bugzilla/>
References: <bug-62076-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-08/txt/msg01488.txt.bz2
Content-length: 501

https://gcc.gnu.org/bugzilla/show_bug.cgi?idb076

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|---                         |FIXED

--- Comment #5 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
The test case no longer fails, the fix looks good, so I think we can change
this.

[Bug fortran/62215] Updating .mod files on Win32 fails

[jeffrey.armstrong at approximatrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #5 from Jeffrey Armstrong <jeffrey.armstrong at approximatrix dot com> ---
While checking for MinGW (or MinGW-W64) (or non-POSIX Cygwin if it's a
thing...) would allow this to be fixed for Windows, it would continue to assume
that any other C runtime would have a POSIX-compliant "rename()" function.  I
would think that could be dangerous. That said, I can't think of another C
runtime that doesn't provide a POSIX-compliant "rename()" function other than
some versions of the OpenWatcom C runtime (I'm pretty sure you guys don't care
about that) and any MS-DOS or OS/2 C runtimes. 

Is there a big cost or danger to "unlink()" the existing file first?  POSIX
"rename()" is going to do that anyway; my patch (and GNU Fortran < 4.9) simply
does the deleting explicitly.

[Bug fortran/62215] Updating .mod files on Win32 fails

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #6 from Janne Blomqvist <jb at gcc dot gnu.org> ---
(In reply to Steve Kargl from comment #3)
> Not having a copyright assignment won't interfere with this
> patch being applied.  I'm waiting for confirmation from Janne
> that I can revert the changes in his previous patch.

I don't remember, but I guess I read the description from the POSIX standard
and assumed non-POSIX systems would behave the same way. But now that I look at
C11 (n1570.pdf) it says that if *new exists the behavior is
implementation-defined.

So please go ahead and commit the patch to trunk and 4.9. 

As for the #ifdef stuff, instead of trying to enumerate all non-POSIX systems
one could go the other way instead, e.g. something like

#if not defined(__unix__) || not defined(_POSIX_VERSION)
... unlink stuff
#endif

(See http://sourceforge.net/p/predef/wiki/Home/ for lists of various
pre-defined macros)

That would save an extra syscall on POSIX systems, but I don't think it really
matters, so do as you see fit.

[Bug fortran/62215] Updating .mod files on Win32 fails

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Janne Blomqvist from comment #6)
> So please go ahead and commit the patch to trunk and 4.9. 

It's fine from my side as well.

> As for the #ifdef stuff, [...]
> That would save an extra syscall on POSIX systems, but I don't think it
> really matters, so do as you see fit.

As it is only in the compiler and not in the libgfortran library (as I somehow
thought it would be), simply reinstating the unlink call is probably best.

[Bug fortran/62215] Updating .mod files on Win32 fails

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #8 from Janne Blomqvist <jb at gcc dot gnu.org> ---
Author: jb
Date: Fri Aug 29 20:46:15 2014
New Revision: 214742

URL: https://gcc.gnu.org/viewcvs?rev=214742&root=gcc&view=rev
Log:
PR 62215 Reinstate unlinking old module file before renaming.

2014-08-29  Jeffrey Armstrong  <jeffrey.armstrong@approximatrix.com>

    PR fortran/62215
    * module.c (gfc_dump_module): Unlink old module file before
    renaming new one.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/module.c

[Bug fortran/62215] Updating .mod files on Win32 fails

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

--- Comment #9 from Janne Blomqvist <jb at gcc dot gnu.org> ---
Author: jb
Date: Fri Aug 29 20:49:16 2014
New Revision: 214743

URL: https://gcc.gnu.org/viewcvs?rev=214743&root=gcc&view=rev
Log:
PR 62215 Unlink old module file before renaming.

2014-08-29  Jeffrey Armstrong  <jeffrey.armstrong@approximatrix.com>

    Backport from trunk
    PR fortran/62215
    * module.c (gfc_dump_module): Unlink old module file before
    renaming new one.

Modified:
    branches/gcc-4_9-branch/gcc/fortran/ChangeLog
    branches/gcc-4_9-branch/gcc/fortran/module.c

[Bug fortran/62215] Updating .mod files on Win32 fails

[jb at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62215

Janne Blomqvist <jb at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #10 from Janne Blomqvist <jb at gcc dot gnu.org> ---
Fixed on trunk and 4.9, closing. Thanks for the report and patch!