Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-09-24 Fir de Conversatie Ken Takata 
Hi,

2012/08/25 Sat 12:41:45 UTC+9 Ken Takata:
 Then I found your post and tried David's patch.
 It seems that the patch works well but I found a little problem.
 This is an updated patch.
 https://gist.github.com/3436380
 
 The differences from David's patch are:
  1. Fix memory leakage in win32_file_is_symbolic_link().
  2. Use mch_stat() to implement mch_getperm().
 (His implementation on Google Code does not support multibyte characters.)
  3. Add support for breakhardlink and breaksymlink.
  4. Change the name of some functions.
 
I have updated the patch.

 1. fix mch_remove() to support multibyte file name again.
 2. fix return value of win32_setattrs().
 3. add some comments.

Best regards,
Ken Takata

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
diff --git a/src/fileio.c b/src/fileio.c
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3778,12 +3778,12 @@
 	}
 	}
 
-# ifdef UNIX
 	/*
 	 * Break symlinks and/or hardlinks if we've been asked to.
 	 */
 	if ((bkc_flags  BKC_BREAKSYMLINK) || (bkc_flags  BKC_BREAKHARDLINK))
 	{
+# ifdef UNIX
 	int	lstat_res;
 
 	lstat_res = mch_lstat((char *)fname, st);
@@ -3799,8 +3799,18 @@
 		 st_old.st_nlink  1
 		 (lstat_res != 0 || st.st_ino == st_old.st_ino))
 		backup_copy = FALSE;
-	}
-#endif
+# elif defined(WIN32)
+	/* Symlinks. */
+	if ((bkc_flags  BKC_BREAKSYMLINK)
+		 mch_is_symbolic_link(fname))
+		backup_copy = FALSE;
+
+	/* Hardlinks. */
+	if ((bkc_flags  BKC_BREAKHARDLINK)
+		 mch_is_hard_link(fname))
+		backup_copy = FALSE;
+# endif
+	}
 
 #endif
 
diff --git a/src/os_win32.c b/src/os_win32.c
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -72,6 +72,16 @@
 # endif
 #endif
 
+/*
+ * Reparse Point
+ */
+#ifndef FILE_ATTRIBUTE_REPARSE_POINT
+# define FILE_ATTRIBUTE_REPARSE_POINT	0x0400
+#endif
+#ifndef IO_REPARSE_TAG_SYMLINK
+# define IO_REPARSE_TAG_SYMLINK		0xA00C
+#endif
+
 /* Record all output and all keyboard  mouse input */
 /* #define MCH_WRITE_DUMP */
 
@@ -208,6 +218,10 @@
 static char *vimrun_path = vimrun ;
 #endif
 
+static int win32_getattrs(char_u *name);
+static int win32_setattrs(char_u *name, int attrs);
+static int win32_set_archive(char_u *name);
+
 #ifndef FEAT_GUI_W32
 static int suppress_winsize = 1;	/* don't fiddle with console */
 #endif
@@ -2568,57 +2582,54 @@
 /*
  * get file permissions for `name'
  * -1 : error
- * else FILE_ATTRIBUTE_* defined in winnt.h
+ * else mode_t
  */
 long
 mch_getperm(char_u *name)
 {
-#ifdef FEAT_MBYTE
-if (enc_codepage = 0  (int)GetACP() != enc_codepage)
-{
-	WCHAR	*p = enc_to_utf16(name, NULL);
-	long	n;
-
-	if (p != NULL)
-	{
-	n = (long)GetFileAttributesW(p);
-	vim_free(p);
-	if (n = 0 || GetLastError() != ERROR_CALL_NOT_IMPLEMENTED)
-		return n;
-	/* Retry with non-wide function (for Windows 98). */
-	}
-}
-#endif
-return (long)GetFileAttributes((char *)name);
+struct stat st;
+int n;
+
+n = mch_stat(name, st);
+return n == 0 ? (int)st.st_mode : -1;
 }
 
 
 /*
  * set file permission for `name' to `perm'
+ *
+ * return FAIL for failure, OK otherwise
  */
 int
 mch_setperm(
 char_u  *name,
 longperm)
 {
-perm |= FILE_ATTRIBUTE_ARCHIVE;	/* file has changed, set archive bit */
+long	n;
 #ifdef FEAT_MBYTE
+WCHAR *p;
 if (enc_codepage = 0  (int)GetACP() != enc_codepage)
 {
-	WCHAR	*p = enc_to_utf16(name, NULL);
-	long	n;
+	p = enc_to_utf16(name, NULL);
 
 	if (p != NULL)
 	{
-	n = (long)SetFileAttributesW(p, perm);
+	n = _wchmod(p, perm);
 	vim_free(p);
-	if (n || GetLastError() != ERROR_CALL_NOT_IMPLEMENTED)
-		return n ? OK : FAIL;
+	if (n == -1  GetLastError() != ERROR_CALL_NOT_IMPLEMENTED)
+		return FAIL;
 	/* Retry with non-wide function (for Windows 98). */
 	}
 }
+if (p == NULL)
 #endif
-return SetFileAttributes((char *)name, perm) ? OK : FAIL;
+	n = _chmod(name, perm);
+if (n == -1)
+	return FAIL;
+
+win32_set_archive(name);
+
+return OK;
 }
 
 /*
@@ -2627,49 +2638,12 @@
 void
 mch_hide(char_u *name)
 {
-int		perm;
-#ifdef FEAT_MBYTE
-WCHAR	*p = NULL;
-
-if (enc_codepage = 0  (int)GetACP() != enc_codepage)
-	p = enc_to_utf16(name, NULL);
-#endif
-
-#ifdef FEAT_MBYTE
-if (p != NULL)
-{
-	perm = GetFileAttributesW(p);
-	if (perm  0  GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)
-	{
-	/* Retry with non-wide function (for Windows 98). */
-	vim_free(p);
-	p = NULL;
-	}
-}
-if (p == NULL)
-#endif
-	perm = GetFileAttributes((char *)name);
-if (perm = 0)
-{
-	perm |= FILE_ATTRIBUTE_HIDDEN;
-#ifdef FEAT_MBYTE
-	if (p != NULL)
-	{
-	if (SetFileAttributesW(p, perm) == 0
-		 GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)
-	{
-		/* Retry with non-wide function (for Windows 98). */
-		vim_free(p);
-		p = NULL;
-	}
-	}
-	if

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-08-22 Fir de Conversatie Ian Halliday 
What ever happened to this patch?

I just ran into this problem for a second time in the last year, having 
forgotten how to fix it since last time, had to look everything up again.  A 
fix in the main branch would be great to have.

Ian

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-29 Fir de Conversatie Ben Fritz 
On Thursday, March 29, 2012 12:52:49 AM UTC-5, David Pope wrote:
 Thanks, any help is appreciated.  I have updated the patch with some further 
 fixes (it broke a common pattern for calling mch_getperms to check for file 
 existence).  The latest patch is available in my fork at github: 
 https://code.google.WBRcom/r/depope-vim/ .  Should I continue posting 
 patches here in vim_dev or is github considered better?
 
 

That's not github, by the way :-)

It can be nice to pull from a real repository, but some people (me included) 
would prefer to put experimental patches into a patch queue with mq. I like 
this option better because Bram doesn't pull from clones, he just uses the 
patches (or at least this is the case for the official repository, obviously 
I don't know what his personal clones might look like). So, if I pull a bunch 
of changes on the default branch into my clone, that history never gets pulled 
in and either I will need to strip it or let some extra head just dangle.

So if you want to commit to a clone, that's good, and you should share it...but 
certainly continue posting patches on vim_dev. I wonder if there's a good way 
to share patch queues related to a project? That might be an even better way.

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-29 Fir de Conversatie David Pope 
On Thu, Mar 29, 2012 at 10:30 AM, Ben Fritz fritzophre...@gmail.com wrote:


 That's not github, by the way :-)


oops :)


 It can be nice to pull from a real repository, but some people (me
 included) would prefer to put experimental patches into a patch queue with
 mq. I like this option better because Bram doesn't pull from clones, he
 just uses the patches (or at least this is the case for the official
 repository, obviously I don't know what his personal clones might look
 like). So, if I pull a bunch of changes on the default branch into my
 clone, that history never gets pulled in and either I will need to strip it
 or let some extra head just dangle.


So if you want to commit to a clone, that's good, and you should share
 it...but certainly continue posting patches on vim_dev. I wonder if there's
 a good way to share patch queues related to a project? That might be an
 even better way.


Good point; I haven't used the mq extension before, so I'll go figure it
out, along with its implied workflows.  I'll probably have questions after
that.  ;)

Thanks,
-- Dave

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-28 Fir de Conversatie David Pope 
On Mon, Mar 26, 2012 at 11:00 AM, Benjamin Fritz fritzophre...@gmail.comwrote:


 I'd like to test, but currently the only computers I use are stuck on
 Windows XP and I'm not able to. The machine I was using at the time I
 originally encountered the issue was a dual-boot Windows Vista/Ubuntu
 machine, which now has a bad motherboard.


Thanks, any help is appreciated.  I have updated the patch with some
further fixes (it broke a common pattern for calling mch_getperms to check
for file existence).  The latest patch is available in my fork at github:
https://code.google.com/r/depope-vim/ .  Should I continue posting patches
here in vim_dev or is github considered better?

-- Dave

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-26 Fir de Conversatie Benjamin Fritz 
On Fri, Mar 23, 2012 at 11:35 PM, David Pope d.e.p...@gmail.com wrote:


 Is there any feedback on this patch, or on the way the fix was presented?
  As I said earlier, I'm new to vim development so I'm all ears.  :)  Is
 there someone I need to direct this patch toward, e.g. someone who deals
 primarily with the Windows version of vim, to get it vetted/updated for
 mainline inclusion?


I'd like to test, but currently the only computers I use are stuck on
Windows XP and I'm not able to. The machine I was using at the time I
originally encountered the issue was a dual-boot Windows Vista/Ubuntu
machine, which now has a bad motherboard.

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Re: [Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-23 Fir de Conversatie David Pope 
Hello Bram, Benjamin, and all,

On Wed, Mar 21, 2012 at 2:27 AM, David Pope d.e.p...@gmail.com wrote:

  A patch definitely helps.  And a way to reproduce the problem.
 

 Reproduction is easy.

 1. Create a symbolic link on Windows. (e.g. mklink link_path target_path)
 2. open the file in Vim
 3. write the file from Vim

 The symbolic link has been destroyed, now it's a real file separate
 from the file it was originally linked to.


 Here's my first shot at a patch that fixes the can't save to symlinks
 bug on Windows.

snip


Is there any feedback on this patch, or on the way the fix was presented?
 As I said earlier, I'm new to vim development so I'm all ears.  :)  Is
there someone I need to direct this patch toward, e.g. someone who deals
primarily with the Windows version of vim, to get it vetted/updated for
mainline inclusion?

Thanks,

David Pope

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

[Patch] Re: 'backupcopy' and Windows Vista symbolic links

2012-03-21 Fir de Conversatie David Pope 
On Thu, Mar 15, 2012 at 5:02 PM, Benjamin Fritz fritzophre...@gmail.comwrote:

 On Thu, Mar 15, 2012 at 2:28 PM, Bram Moolenaar b...@moolenaar.net
 wrote:
 
  David Pope wrote:
 
  On Wednesday, July 20, 2011 3:54:09 PM UTC-4, Craig Barkhouse wrote:
 
   The mch_is_linked() function in os_win32.c only checks if there is
   more than one hard link (i.e. name) for the file.  It doesn't check
   if the file is a symbolic link.  By contrast the Unix code does
   check if the file is a symbolic link.
  
   Sounds like a TODO item.
 
  Hello all, did this ever get turned into a TODO item?  I've
  encountered this myself (I'm syncing all my vim configuration across
  machines using Dropbox, with symbolic links for .vim/, .vimrc, and
  .gvimrc).
 
  I see in the latest Mercurial code that mch_is_linked() still only
  checks for hard links.  If it's not already in someone's TODO bucket I
  can work on a fix.  The APIs are straightforward in the versions of
  Windows that support symlinks; I suspect more of the work will be in
  figuring out how to get that support into vim without breaking binary
  compatibility on older systems.  Would the maintainers be interested
  in seeing a patch for this?
 
  A patch definitely helps.  And a way to reproduce the problem.
 

 Reproduction is easy.

 1. Create a symbolic link on Windows. (e.g. mklink link_path target_path)
 2. open the file in Vim
 3. write the file from Vim

 The symbolic link has been destroyed, now it's a real file separate
 from the file it was originally linked to.


Here's my first shot at a patch that fixes the can't save to symlinks bug
on Windows.  It augments the Windows version of mch_is_linked() to also
return TRUE if the file is a symbolic link, so the delete-then-move pattern
is avoided and the symlink is preserved.

The patch also addresses a separate bug I encountered while fixing the
above.  If you set nowritebackup on Windows, and then save a file that you
opened via a symbolic link, the readonly attribute gets set.

The cause was that the Windows version of mch_getperm() was returning
Windows FILE_ATTRIBUTE_* flags instead of the Unix-style flags in mode_t,
which was then later passed unchanged to the CRT open() function (which
expects mode_t flags).

For a normal (non-symlink, non-whatever) file, this just happened to be
FILE_ATTRIBUTE_NORMAL (0200, 0x80), which maps to the Unix S_IWUSR.  By
sheer chance, this meant that normal files correctly received write
permission, i.e. no readonly flag.

For a symlink, FILE_ATTRIBUTE_NORMAL is not set; for example, the symlink I
was testing against returned FILE_ATTRIBUTE_ARCHIVE (040, 0x20) and
FILE_ATTRIBUTE_REPARSE_POINT (02000, 0x400).  By the time these made it
back to create(), it appeared as though we wanted no write permissions,
i.e. set the readonly flag.

The patch changes os_win32.c so that all code outside of the file deals
with mode_t flags, while os_win32.c itself deals with FILE_ATTRIBUTE_*
flags.  There are a couple of new internal helper functions for this.

This is my first patch, so I'm sure I messed up somewhere; feedback is
welcome.  Sorry if the above is too wordy, this was a fun one to diagnose.
 ;)

Caveats:
  * I have no way to test Win98, can someone help?  There is lots of Win98
fallback code in os_win32.c (to use non-Unicode functions when FEAT_MBYTE
is defined).
  * I added win32_* signatures to os_win32.pro for the new internal
functions; I'm not sure if the *.pro files are intended mainly for function
exports, or just to avoid having to make forward declarations (as I've done
here).
  * I'm using a couple of CRT functions that weren't in use before; I don't
know if it's reasonable to assume that they work as expected on all the old
platforms that are required.


David Pope

-- 
You received this message from the vim_dev maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php


symlinkfix.diff
Description: Binary data