[pacman-dev] updpkgsums read/write collision
Hi all. I have noticed a problem with updpkgsums on Windows when it's run on a subst'd drive (created with `subst` or using an entry in `DOS Devices` registry key). The error message I see is following: /usr/bin/updpkgsums: line 85: PKGBUILD: Permission denied The PKGBUILD is then gone. I don't fully understand what's going on on lines 78-97 with the input/output redirection and deleting, but I assume this can be fixed by 1) either enhancing the routine for dereferencing symlinks (lines 54-62) to recognize subst'd drives, 2) or by modifying the script to first rename the original $buildfile, and delete it after finishing the new $buildfile. For the time being, I worked around the problem by modifying the script to first read $buildfile into a variable, then delete it and then pass the variable contents to awk to write to $buildfile. My question is following: can the script be changed in order to avoid this issue? Ref: https://github.com/Alexpux/MSYS2-packages/issues/143 -- David Macek
On Sun, Dec 14, 2014 at 05:13:11PM +0100, David Macek wrote:
Hi all.
I have noticed a problem with updpkgsums on Windows when it's run on a subst'd drive (created with `subst` or using an entry in `DOS Devices` registry key). The error message I see is following:
/usr/bin/updpkgsums: line 85: PKGBUILD: Permission denied
The PKGBUILD is then gone.
I don't really understand this. You're saying that the PKGBUILD itself isn't subst'd, but the underlying device that it resides on? That sounds an awful lot like a deficiency in the underlying implementation (or some problem with cygwin's handling of such drives).
I don't fully understand what's going on on lines 78-97 with the input/output redirection and deleting
The redirection on line 97 keeps an open FD to the original file, allowing us to delete it without actually dropping the file's link count to 0. We can then modify the stream through awk, and write back to the original file. Doing this directly, i.e. awk PROGRAM file >file ...doesn't work, because you're truncating the file before reading it.
but I assume this can be fixed by 1) either enhancing the routine for dereferencing symlinks (lines 54-62) to recognize subst'd drives,
Not having used Windows as a day-to-day OS in last 5-6 years, I have no idea how to do this, nor can I easily test it. Patches welcome for such an approach.
2) or by modifying the script to first rename the original $buildfile, and delete it after finishing the new $buildfile.
So, the implemenation is the way it is right now because I didn't want to play games with temporary files (and the naming problems that come along with it). I already conceded to making a tempdir for the purposes of keeping the build directory clean, so I suppose I can be persuaded to use a tempfile for the newly rewritten PKGBUILD. Does this patch fix your problem? https://paste.xinu.at/oKQMN/
For the time being, I worked around the problem by modifying the script to first read $buildfile into a variable, then delete it and then pass the variable contents to awk to write to $buildfile.
My question is following: can the script be changed in order to avoid this issue?
On 14. 12. 2014 17:53, Dave Reisner wrote:
I don't really understand this. You're saying that the PKGBUILD itself isn't subst'd, but the underlying device that it resides on? That sounds an awful lot like a deficiency in the underlying implementation (or some problem with cygwin's handling of such drives).
It's possible. I'll present this issue to cygwin devs.
Not having used Windows as a day-to-day OS in last 5-6 years, I have no idea how to do this, nor can I easily test it. Patches welcome for such an approach.
The only solution I found so far was to use WINAPI QueryDosDevice, which I don't think is appropriate for pacman as a cross-platform application that doesn't directly deal with drives and filesystems.
So, the implemenation is the way it is right now because I didn't want to play games with temporary files (and the naming problems that come along with it). I already conceded to making a tempdir for the purposes of keeping the build directory clean, so I suppose I can be persuaded to use a tempfile for the newly rewritten PKGBUILD.
Does this patch fix your problem?
Yes, it does. It would help if it was merged into upstream pacman. -- David Macek
On Sun, Dec 14, 2014 at 06:40:38PM +0100, David Macek wrote:
On 14. 12. 2014 17:53, Dave Reisner wrote:
I don't really understand this. You're saying that the PKGBUILD itself isn't subst'd, but the underlying device that it resides on? That sounds an awful lot like a deficiency in the underlying implementation (or some problem with cygwin's handling of such drives).
It's possible. I'll present this issue to cygwin devs.
I'd be curious to know if it behaves the same way as root (assuming such a concept exists). It should be sufficient the present the minimal case of: $ echo 'hello world' >foo $ { rm foo; awk '1' >foo; } <foo Technically, I don't believe the file needs content, but as a matter of verification, I include it. The file will appear unmodified afterwards if this is working as intended.
Not having used Windows as a day-to-day OS in last 5-6 years, I have no idea how to do this, nor can I easily test it. Patches welcome for such an approach.
The only solution I found so far was to use WINAPI QueryDosDevice, which I don't think is appropriate for pacman as a cross-platform application that doesn't directly deal with drives and filesystems.
So, the implemenation is the way it is right now because I didn't want to play games with temporary files (and the naming problems that come along with it). I already conceded to making a tempdir for the purposes of keeping the build directory clean, so I suppose I can be persuaded to use a tempfile for the newly rewritten PKGBUILD.
Does this patch fix your problem?
Yes, it does. It would help if it was merged into upstream pacman.
Surely. This is an acceptable change for Linux as well. Thanks for the report. Cheers, dR
On 14. 12. 2014 19:01, Dave Reisner wrote:
I'd be curious to know if it behaves the same way as root (assuming such a concept exists).
The error still occurs when running the shell with administrator privileges.
It should be sufficient the present the minimal case of:
$ echo 'hello world' >foo $ { rm foo; awk '1' >foo; } <foo
Technically, I don't believe the file needs content, but as a matter of verification, I include it. The file will appear unmodified afterwards if this is working as intended.
I simplified it even more. In case you're interested, see <https://gist.github.com/elieux/6463521192baed613099>. I'm currently discussing it on #msys2.
Surely. This is an acceptable change for Linux as well. Thanks for the report.
Thanks. -- David Macek
participants (2)
-
Dave Reisner
-
David Macek