Re: Subversion checked-out files not indexed in Windows search
On 3/17/2014 5:40 AM, Bert Huijben wrote: And updating these attributes one at a time during checkout would be huge (not a small) performance killer during checkout, as both operations would be updating the MFT. We should try to perform as much operations as possible while having the files open anyway, as each file open (which at the kernel level includes operations as setting properties) will involve virus scanners, etc. With the current trunk code the best solution might be to just remove the setting of the 'don't index' property, as the current code already moves the files in place while still open and 100% locked from indexers. (Those indexers really slowed us down on Windows XP and 7 around the time that we introduced this flag as the indexer opened the file between us creating and moving the file) Bert As far as the ACLs, I think Windows Explorer uses this technique, so if it causes problems with scanners, these problems happen just working with files in Explorer too. It is more complexity, but you could check if the 'attributes' are incorrect and only make the API call if there is something to change. Thus, only those users affected by the problem incur any decrease in performance. (This is assuming the API's in question are not already optimized for this case; if I recall correctly, I don't think these kinds of APIs edit the modified date in any case, for example.)
Re: Subversion checked-out files not indexed in Windows search
On 3/13/2014 9:08 AM, Branko Čibej wrote:
I understand that; it simply means that when we install a file from
.svn/tmp into the working copy proper, we have to change that
attribute; either before or after the file is moved into place.
When this came up on ACLs, there seemed to be a lot of resistance to
changing the file after it was in place. Kind of switching the subject,
but if you're okay with changing files after they are in place, consider
also propagating the ACL at that time too. As has been pointed out,
below is not the best possible solution as the file momentarily has the
undesired (but current-behavior) ACL and so there could be a problem on
abort. But I think it is an improvement over the current behavior, and
it is simple to implement.
This seems to give the desired effect:
DWORD ret;
ACL emptyAcl;
if (!InitializeAcl(&emptyAcl, sizeof(ACL), ACL_REVISION)) {
return 1;
}
// UNPROTECTED_SACL_SECURITY_INFORMATION ensures to enable inheritance.
// An empty DACL must be specified, not a NULL pointer.
ret = SetNamedSecurityInfo(fileName, SE_FILE_OBJECT,
DACL_SECURITY_INFORMATION | UNPROTECTED_SACL_SECURITY_INFORMATION,
NULL, NULL, &emptyAcl, NULL);
if (ret != ERROR_SUCCESS) {
return 1;
}
I think also a check that the file supports ACLs (e.g., because FAT/etc.
systems do not).
Thanks, I'll try not to harp on this subject again in the future. It's
just the fact that SVN does not do this (even as an option that can be
turned on) happens to be my #1 issue with svn.
Re: Respecting inherited Windows file permissions on file create
> Subject: Re: Respecting inherited Windows file permissions on file > create > From: Branko Čibej > Date: Mon, September 30, 2013 6:07 am > Whew. I'd really prefer not to second-guess Windows ACL inheritance like > this, Not to beat a dead horse, but if you call SetNamedSecurityInfo after the move, then most of the time things will end up "right" and once in a while they will end up no worse than what you have today. I don't know if SetNamedSecurityInfo is optimized for the case where it performs a no op, but you can code that optimization yourself to avoid any performance regression. (Have not researched how easy that would be. Nor the corner case where you have (F) Full Control access to .svn directory but not the final directory. Though as the file's creator/owner I think you still will be able to effect the ACL change.) (Sorry, I haven't figured out why my mail program Reply to All doesn't work quite right yet...)
Re: Respecting inherited Windows file permissions on file create
> From: Branko ÄŒibej > Date: Sun, 29 Sep 2013 07:14:07 +0200 > This is not in fact a bug in Subversion > The solution that jiggles the security descriptor is not acceptable. I note that svn is not touching the place where the inherit is actually assigned (a directory), yet it is interfering with it. This isn't a nice thing to do on Windows. It is nearly introducing an "inconsistency" in the file system. Just my opinion, but I feel that any time this occurs on Windows may be a bug. Whatever you call it, the effect on the user is the same. Actually, I thought it was a bug in the Windows OS until I came across the blog post I referenced. (If this all doesn't shock you, you might not be a Windows programmer!) If you want to be permissions agnostic on Windows, then I think you will want the file to end up with the default creation permissions, instead of arguably inconsistent ones that arise from the move gotcha. I think this would be an improvement from being permission hostile in this case, and ease scenarios which I hope should be valid, such as running a web server against parts of the working copy. Like svn, most Windows programs do not concern themselves with permissions. The right thing" just happens by default. The solution Ivan mentioned of creating a temp file in the same directory as the final file is common (e.g., Microsoft Office. See "Data Integrity" and "Saved Files (Same Directory as the Saved File)" in http://support.microsoft.com/kb/211632). That way, of course, has its pros and cons, too. There are numerous ways to address this. Crossing fingers that some of them are deemed acceptable and worth the effort. I did file issue #4432 as instructed. (Sorry about the double spacing due to me guessing wrong about how that text box worked.) Thanks for your consideration. I will be happy to answer any questions you might have.
Re: Respecting inherited Windows file permissions on file create
> Subject: Re: Respecting inherited Windows file permissions on file create > From: Ivan Zhakov > Date: Sat, September 28, 2013 3:29 pm > Solution (2) is create temporary file in > wc\a folder. In this case it will get proper inherited permissions > from wc\a folder. You may try this solution in your test by modified > permissions of wc\.svn\tmp directory. Okay, now I see. That sounds like it would work, but would it make any patch more or less invasive than it needs to be? I really don't mean to argue, but you seem to have not considered if inserting a SetNamedSecurityInfo API call right after the file move call would work (and be less invasive than other solutions), as the Microsoft blogger I referenced previously described. But hey if the tests pass, why should I complain! I could further describe ways to make this work "correctly" using Windows APIs, including the issue with permissions that were assigned directly on the file itself. These are probably not going to fit so cleanly with the abstraction layer in place. And they are not going to be as simple as calling one function with a bunch of NULL params as would be the case for SetNamedSecurityInfo. Note, it is not feasible for me to set up a Windows svn development environment to actually work on the svn code myself. So I would like some guidance as to how to best assist with this, or if necessary better explain this issue. (i.e., if I haven't adequately explained that it is bad form to leave inheritable permissions unpropagated on Windows. The Explorer shell has poor support for this case. Users can see surprising things if they try to troubleshoot merely through the Explorer GUI.)
RE: Respecting inherited Windows file permissions on file create
> From: Ivan Zhakov > Date: Sat, September 28, 2013 2:46 pm > I see two possible solutions for this problem: > 1. Use hTemplate argument in CreateFile call when temporary file is created > 2. Created temporary file along original file to be replaced instead > of .svn/tmp in working copy root > > Option (1) is hard to because APR abstraction, while (2) should be > easier but solve only problem when file itself doesn't have specific > permissions. I think you are describing solutions to a different (harder) problem than I raised. You are trying to solve the case where the "disrespected" permissions were ones that were applied directly to the file that was later replaced by svn. My case was permissions implied by inheritance. I think that takes a different solution. I say that yours is harder, because the problem I described is solvable by adding a single call to SetNamedSecurityInfo with parameters whose values are easy to determine (i.e., mostly NULLs). If I follow correctly, neither of your two solutions would cause my sample reproduction code to succeed. What I would expect is defined to be identical to what would happen if svn always recreated files from scratch at their final location using default options, without any filesystem "move" calls. (Whether that is, in fact, a viable option to allow the user to configure may be another angle to consider.) For what it is worth, it is less common to assign permissions on individual files than to inherit them from directories. But in either case, I think inserting the call to SetNamedSecurtyInfo is an improvement. Users should be more understanding that the permissions were reset to inheritance than they were copied from some hidden place without respect to inheritance defined on the file's final location parent directory permissions. If I understand correctly, use of SetNamedSecurityInfo will cause the system to behave as if the move optimization (I'm assuming it is some kind of optimization) was not in place and that the file was instead written from scratch with the default security parameters on Windows.
