Re: [PATCH] [Issue 870] import should set svn:executable automatically

Brian Denny <brian@briandenny.net> writes:

> just cuz it seemed like a good one to get my feet wet with. :)

Woo! Attack that bite-sized task!

>
> notes/questions:
> - i assume that correct behavior is to set the svn:executable if
> the file is actually executable by the user running svn. is
> this correct? (as opposed to, for example, if *any* of the
> executable bits are set).

Sounds reasonable to me.

> - i created a helper method "is_executable" to determine if given
> file is executable by current user. it seems like a pretty
> general-purpose utility; but it's not currently used anywhere
> other than this one place. should (things like) this go in the
> public interface?

Looks good in your patch below; maybe put it into svn_io_*, in
libsvn_subr?

> - it seems like new tests should be written for this functionality,
> but i don't yet grok the existing test code well enough to be able
> to work this in. maybe in a few days. let me know if you wish to
> wait for the tests before accepting the patch.

We'd love to help you with tests. The python test framework isn't
that complex, really. The issue is figgering out how to get python to
set +x in an os-portable way, I'm guessing.

> - there is a note in the issue tracker that this mechanism should
> defer to the mechanism described in issue 869 where applicable.
> as issue 869 has not been done, this is not addressed by the patch.

Your patch doesn't need to wait for issue 869. When somebody decides
to do 869, they can make your is_executable() routine smarter.

Your patch looks reasonable to me; does it work with your hand-tests?

Does it pass 'make check'?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 25 18:57:14 2002