Changing the svn commit hook to allow tabs for tests.

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Changing the svn commit hook to allow tabs for tests.

Keith Miller
Hi all,

I don’t know if this is possible but it would be great if some sub-directories could be excluded from the no-tabs pre-commit hook. For example, there are many test262 tests that contain tabs, either because they intentionally try to test parsing tab characters or because they were written by someone that used them. Regardless, it’s pretty inconvenient to add the svn attribute that allows tabs every time I update the tests. Since svn properties can’t apply to directories, as far as I know, there’s no way to add the “allow-tabs” property to the entire test262 directory.

Alternatively, we could remove the pre-commit hook as tabs should be caught by the style-checker anyway. This is a more contentious choice so I’m not proposing it as the first option unless someone else wants to push for it.

Cheers,
Keith Miller
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changing the svn commit hook to allow tabs for tests.

Darin Adler
> On May 10, 2019, at 1:00 PM, Keith Miller <[hidden email]> wrote:
>
> I don’t know if this is possible but it would be great if some sub-directories could be excluded from the no-tabs pre-commit hook.

Maybe we can rewrite the pre-commit hook to allow a whole-directory exception. Ideally I’d prefer not to hardcode directories.

> it’s pretty inconvenient to add the svn attribute that allows tabs every time I update the tests

Does it really have to be inconvenient? Can we make script that does this and check it in so anyone can run it? Or build it into webkit-patch or whatever tool you already use?

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changing the svn commit hook to allow tabs for tests.

Keith Miller


> On May 10, 2019, at 1:07 PM, Darin Adler <[hidden email]> wrote:
>
>> On May 10, 2019, at 1:00 PM, Keith Miller <[hidden email]> wrote:
>>
>> I don’t know if this is possible but it would be great if some sub-directories could be excluded from the no-tabs pre-commit hook.
>
> Maybe we can rewrite the pre-commit hook to allow a whole-directory exception. Ideally I’d prefer not to hardcode directories.

I’m not sure I know what you mean by allow a whole-directory exception. Do you mean a top level directory? Or some kind of parameter we pass to the hook to ignore some directory for that run?

I was thinking about excluding OpenSource/JSTests/ and maybe OpenSource/LayoutTests/ as these are the two directories where I expect all, if not most, tests that have tabs to live.

>
>> it’s pretty inconvenient to add the svn attribute that allows tabs every time I update the tests
>
> Does it really have to be inconvenient? Can we make script that does this and check it in so anyone can run it? Or build it into webkit-patch or whatever tool you already use?

We could do this. One remaining problem, however, is that you can’t commit with git-svn as it doesn’t support svn properties (or at least I wasn’t able to figure it out).

Cheers,
Keith

>
> — Darin

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changing the svn commit hook to allow tabs for tests.

Darin Adler
> On May 10, 2019, at 1:13 PM, Keith Miller <[hidden email]> wrote:
>
> I’m not sure I know what you mean by allow a whole-directory exception. Do you mean a top level directory? Or some kind of parameter we pass to the hook to ignore some directory for that run?

I meant that we could add something the pre-commit hook could see in Subversion that would create an exception for a whole directory, rather than something inside the hook itself. Perhaps a specially named file, or a Subversion attribute on a specially named file, or something more clever. If Subversion had attributes on directories, it could be that.

> you can’t commit with git-svn as it doesn’t support svn properties (or at least I wasn’t able to figure it out)

Ah, that’s a big blocker if lots of people are using git-svn — I certainly use it.

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changing the svn commit hook to allow tabs for tests.

Alexey Proskuryakov-4

10 мая 2019 г., в 13:50, Darin Adler <[hidden email]> написал(а):

On May 10, 2019, at 1:13 PM, Keith Miller <[hidden email]> wrote:

I’m not sure I know what you mean by allow a whole-directory exception. Do you mean a top level directory? Or some kind of parameter we pass to the hook to ignore some directory for that run?

I meant that we could add something the pre-commit hook could see in Subversion that would create an exception for a whole directory, rather than something inside the hook itself. Perhaps a specially named file, or a Subversion attribute on a specially named file, or something more clever. If Subversion had attributes on directories, it could be that.

Subversion supports properties on directories, we use those for svn:ignore as an example. It is correct that the pre-commit hook doesn't currently check parent directory properties.

An alternative is to just set it on all files in the directory.

you can’t commit with git-svn as it doesn’t support svn properties (or at least I wasn’t able to figure it out)

Ah, that’s a big blocker if lots of people are using git-svn — I certainly use it.

Looks like it may now work with recent versions of git (as in since 2015), https://stackoverflow.com/questions/1271449/how-to-set-subversion-properties-with-git-svn :

git-svn: support for git-svn propset

This change allows git-svn to support setting subversion properties.

It is useful for manually setting properties when committing to a subversion repo that requiresproperties to be set without requiring moving your changeset to separate subversion checkout in order to set props.

There is a nit to point out: the code does not support adding props unless there are also content changes to the files as well.
This is demonstrated in the testcase.

- Alexey

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev



_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|

Re: Changing the svn commit hook to allow tabs for tests.

Keith Miller
Resurrecting this from the dead because it came up in: https://bugs.webkit.org/show_bug.cgi?id=209979

On May 10, 2019, at 5:34 PM, Alexey Proskuryakov <[hidden email]> wrote:


10 мая 2019 г., в 13:50, Darin Adler <[hidden email]> написал(а):

On May 10, 2019, at 1:13 PM, Keith Miller <[hidden email]> wrote:

I’m not sure I know what you mean by allow a whole-directory exception. Do you mean a top level directory? Or some kind of parameter we pass to the hook to ignore some directory for that run?

I meant that we could add something the pre-commit hook could see in Subversion that would create an exception for a whole directory, rather than something inside the hook itself. Perhaps a specially named file, or a Subversion attribute on a specially named file, or something more clever. If Subversion had attributes on directories, it could be that.

Subversion supports properties on directories, we use those for svn:ignore as an example. It is correct that the pre-commit hook doesn't currently check parent directory properties.

An alternative is to just set it on all files in the directory.

Cool, where is the commit hook located? Ideally, the commit hook would check recursively if any of the directories below the file (or the file itself) have the allow-tabs property. If that’s too slow or otherwise problematic, just the containing directory would also be reasonable. Thoughts?

Cheers,
Keith


you can’t commit with git-svn as it doesn’t support svn properties (or at least I wasn’t able to figure it out)

Ah, that’s a big blocker if lots of people are using git-svn — I certainly use it.

Looks like it may now work with recent versions of git (as in since 2015), https://stackoverflow.com/questions/1271449/how-to-set-subversion-properties-with-git-svn :

git-svn: support for git-svn propset

This change allows git-svn to support setting subversion properties.

It is useful for manually setting properties when committing to a subversion repo that requiresproperties to be set without requiring moving your changeset to separate subversion checkout in order to set props.

There is a nit to point out: the code does not support adding props unless there are also content changes to the files as well.
This is demonstrated in the testcase.

- Alexey

— Darin
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev




_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev