Re: svn commit: r1845013 - /subversion/trunk/subversion/tests/cmdline/basic_tests.py
Branko Čibej wrote on Tue, 30 Oct 2018 01:04 +0100:
> On 29.10.2018 19:08, Daniel Shahaf wrote:
> > Good morning Brane,
> >
> > br...@apache.org wrote on Sun, 28 Oct 2018 10:40 +:
> >> Add XFAIL tests for issue #4530.
> >>
> >> +++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Sun Oct 28
> >> 10:40:04 2018
> >> @@ -3050,6 +3050,33 @@ def peg_rev_on_non_existent_wc_path(sbox
> >> +def do_move_with_at_signs(sbox, src, dst, dst_cmdline):
> >> + sbox.build()
> >> +
> >> + expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
> >> + expected_status.tweak(src, status='D ', moved_to=dst)
> >> + expected_status.add({dst: Item(status='A ', copied='+',
> >> + moved_from=src, wc_rev='-')})
> >> +
> >> + sbox.simple_move(src, dst_cmdline)
> >> + svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
> >> +
> >> +@Issue(4530)
> >> +@XFail()
> >> +def move_to_target_with_leading_at_sign(sbox):
> >> + "rename to dir/@file"
> >> +
> >> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon')
> >> +
> >> +
> >> +@Issue(4530)
> >> +@XFail()
> >> +def move_to_target_with_leading_and_trailing_at_sign(sbox):
> >> + "rename to dir/@file@"
> >> +
> >> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon@')
> > I'm not actually sure these are supposed to work as these tests expect:
> > supporting A/@upsilon as the first argument like this would prevent us
> > from adding "foo@upsilon" as a command-line syntax for '"foo" at peg
> > revision svn_opt_revision_working' in a future minor release. I think
> > this line of thought applies to the second argument as well.
>
> You must have misread what the test does. A/@upsilon (and A/@upsilon@)
> are the _second_ arguments to 'svn rename'.
Indeed I have. Apologies.
> The command tested is one of:
>
> * 'svn rename iota A/@upsilon' (whch creates A@upsilon, which is wrong)
Agreed.
> * 'svn rename iota A/@upsilont@' (which creates A/@upsilon@ which is
> also wrong)
>
Agreed.
> So there's no way to rename iota to A/@upsilon. We effectively forbd
> renames to fiels with leading @-signs, unless one jumps through a lot of
> hoops. Consider this case, trying to do the rename in the same directory:
>
> $ svn mv @bar@ @qux
> svn: E125001: '@qux' is just a peg revision. Maybe try '@qux@' instead?
> $ svn mv @bar@ @qux@
> A @qux@
> D @bar
>
>
> Notice how the code complains about bare '@qux' but doesn't about
> 'foo/@qux'. There are so many bugs in those few lines of code that I've
> not even counted them all yet.
"@qux" should be equivalent to ".@qux", I assume? Since "." is canonicalized
to "".
> > I can see a number of options:
> >
> > - All PATH arguments (and all TARGET formal arguments when the actual
> > argument isn't a URI) are parsed for peg revisions, no exceptions.
>
> They seem to be, but edge cases are handled incorrectly.
+1
Thanks for the patient answer.
Daniel
Re: Issue with 'svn diff' when file name starts with @
On 29.10.2018 17:08, Tim Van Holder wrote: > (I tried searching the issue tracker but it was extremely unresponsive; > apologies if this is already in there (which seems likely)) > > Initially encountered on Windows (most recent TortoiseSVN). > Reproduced on Linux (Debian testing, subversion-1.10.3-1). > > The help for svn diff suggests that it supports peg revisions (TARGET[@REV]). > However, this does not seem to be the case. > > In fact, while most (all?) other svn commands will treat 'foo' and 'foo@' as > equivalent file names, svn diff does not; 'svn diff foo@' (when only 'foo' > exists) will result in E155010 (the node was not found). > (This seems part of SVN-3231.) > > In addition, there is a significant bug in handling of file names starting > with @; a diff for them can only be acquired by running 'svn diff' in their > directory and using './'. > (I don't immediately see a ticket for this case.) > > Reproduction case: create a new repo containing 2 files: foo/@bar and > foo/xy@zzy > Check out the repository (messages below will assume it is in /wc): > > $ cd /wc > $ svn diff foo/@bar > svn: E155010: The node '/wc/foo@bar' was not found. > $ svn diff foo/@bar@ > svn: E155010: The node '/wc/foo@bar@' was not found. > $ svn diff foo/xy@zzy > $ svn diff foo/xy@zzy@ > svn: E155010: The node '/wc/foo/xy@zzy@' was not found. > $ cd foo > $ svn diff @bar > svn: E125001: '@bar' is just a peg revision. Maybe try '@bar@' instead? > $ svn diff @bar@ > svn: E155010: The node '/wc/foo/@bar' was not found. > $ svn diff ./@bar > $ svn diff ./@bar@ > svn: E155010: The node '/wc/foo/@bar@' was not found. > > So: > > * the normal 'append-@-to-avoid-peg-revision' syntax does not work (but > in some cases the error message suggests using it) > * files containing @ anywhere but at the start work fine > * files with @ at the start seem to lose the directory separator in front > of it, breaking diff operation > * exception: using ./ as path while in the file's directory (so > luckily this means I have a workaround) Hi, You seem to be talking about https://issues.apache.org/jira/browse/SVN-4530 which I've updated recently and started working on. You probably didn't receive an update from Jira about that because the issue was imported from our old tracker at Tigris. Do please add yourself to the watch list, if you're interested. -- Brane P.S: As per [https://subversion.apache.org/reporting-issues.html] discussion belongs on this list, not in Jira.
Re: svn commit: r1845013 - /subversion/trunk/subversion/tests/cmdline/basic_tests.py
On 29.10.2018 19:08, Daniel Shahaf wrote:
> Good morning Brane,
>
> br...@apache.org wrote on Sun, 28 Oct 2018 10:40 +:
>> Add XFAIL tests for issue #4530.
>>
>> +++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Sun Oct 28
>> 10:40:04 2018
>> @@ -3050,6 +3050,33 @@ def peg_rev_on_non_existent_wc_path(sbox
>> +def do_move_with_at_signs(sbox, src, dst, dst_cmdline):
>> + sbox.build()
>> +
>> + expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
>> + expected_status.tweak(src, status='D ', moved_to=dst)
>> + expected_status.add({dst: Item(status='A ', copied='+',
>> + moved_from=src, wc_rev='-')})
>> +
>> + sbox.simple_move(src, dst_cmdline)
>> + svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
>> +
>> +@Issue(4530)
>> +@XFail()
>> +def move_to_target_with_leading_at_sign(sbox):
>> + "rename to dir/@file"
>> +
>> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon')
>> +
>> +
>> +@Issue(4530)
>> +@XFail()
>> +def move_to_target_with_leading_and_trailing_at_sign(sbox):
>> + "rename to dir/@file@"
>> +
>> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon@')
> I'm not actually sure these are supposed to work as these tests expect:
> supporting A/@upsilon as the first argument like this would prevent us
> from adding "foo@upsilon" as a command-line syntax for '"foo" at peg
> revision svn_opt_revision_working' in a future minor release. I think
> this line of thought applies to the second argument as well.
You must have misread what the test does. A/@upsilon (and A/@upsilon@)
are the _second_ arguments to 'svn rename'. They're given twice because
the first is the expected name in the status tree and the second is what
we give SVN on the command line.
The command tested is one of:
* 'svn rename iota A/@upsilon' (whch creates A@upsilon, which is wrong)
* 'svn rename iota A/@upsilont@' (which creates A/@upsilon@ which is
also wrong)
So there's no way to rename iota to A/@upsilon. We effectively forbd
renames to fiels with leading @-signs, unless one jumps through a lot of
hoops. Consider this case, trying to do the rename in the same directory:
$ svn mv @bar@ @qux
svn: E125001: '@qux' is just a peg revision. Maybe try '@qux@' instead?
$ svn mv @bar@ @qux@
A @qux@
D @bar
Notice how the code complains about bare '@qux' but doesn't about
'foo/@qux'. There are so many bugs in those few lines of code that I've
not even counted them all yet.
[[
Addendum:
Why is renaming to A/@upsilon@ in the second test case wrong? because a)
it gives you no way at all to rename to A/@upsilon, and b) an equivalent
'svn add A/@upsilon@' will add A/@upsilon (which must exist of course),
and so will an equivalent 'svn mkdir', where the target directory does
_not_ exist:
$ svn mkdir foo/@x@
A foo/@x
$ svn mkdir foo/@x
-svn: E29: 'foo@x': a peg revision is not allowed here
]]
> It would probably be good to review the list of intended changes before
> time is spent implementing them. Is there such a list? (I don't recall
> seeing one on the issue.)
I'm not implementing any changes, for now I just added tests that
demonstrate the existing bug as reported. That it is a bug should be
obvious and I hope it doesn't need a lot of discussion. I'm really not
that interested in discussing, now, after the fact, how the peg revision
syntax should really work.
> I can see a number of options:
>
> - All PATH arguments (and all TARGET formal arguments when the actual
> argument isn't a URI) are parsed for peg revisions, no exceptions.
They seem to be, but edge cases are handled incorrectly.
> - Ditto, except when the argument denotes an unversioned un-disk path
> (like the targets of «svn add», the file passed as argument to the
> «--targets» option, and so on).
We could never do this in a backwards-compatible way, because all those
arguments are currently processed in the same way.
> - Ditto, plus something that excludes the second target argument to
> «svn move». (I'm not sure about how exactly to do this because the syntax
> is so ambiguous: «svn move file1 file2», «svn move baz foo/bar» where
> «foo/» exists and bar doesn't, and «svn move baz foo/bar» where
> «foo/bar/» is a directory, are all valid.)
>
> I'm sure there are more options, but my point is, let's agree on what
> the CLI documentation on escaping should say in 1.12.
We have always said, "add @ to the end of the name if there's an @ in
the name," no more and no less. I can live with that because it's a
simple and straight-forward rule, *provided* it's actually implemented
correctly. Adding exceptions for move or add or --targets seems like a
cure worse than the disease. It's true that 'svn rename' seems to have
an unintentional exception, but that exception is caused by a bug, it's
not by design.
We really should have created an svn_opt_target_t struct that contained
the path an peg revision as
Re: Subversion 1.11.0 up for testing/signing
On 29.10.2018 15:53, Julian Foad wrote: > The 1.11.0 release artifacts are now available for testing/signing. > Please get the tarballs from > https://dist.apache.org/repos/dist/dev/subversion > and add your signatures there. > > Differences from 1.11.0-rc2 should be limited to the following, so checking > the diff for these should be sufficient -- no need to re-test. > - version number > - CHANGES file > - translations > > It would be really nice if we can get this done ASAP so we can achieve our > planned October release month. +1 to release Unix and Windows. Expected changes match reality. -- Brane
Re: Subversion 1.11.0 up for testing/signing
On Mon, Oct 29, 2018 at 3:54 PM Julian Foad wrote: > > The 1.11.0 release artifacts are now available for testing/signing. > Please get the tarballs from > https://dist.apache.org/repos/dist/dev/subversion > and add your signatures there. > > Differences from 1.11.0-rc2 should be limited to the following, so checking > the diff for these should be sufficient -- no need to re-test. > - version number > - CHANGES file > - translations > > It would be really nice if we can get this done ASAP so we can achieve our > planned October release month. > > Thanks! > > -- > - Julian Verified the differences to 1.11.0-rc2 (only in CHANGES, svn_version.h and translation files), which I tested and signed (Windows). Signature for subversion-1.11.0.zip: -BEGIN PGP SIGNATURE- Version: GnuPG v1 iQIcBAABCgAGBQJb14QnAAoJELWc5tYBDIqtFr4QAI7iMyM9ZQcBWDsniV/akLH1 7icydVoxHigqtC2VXW0Za+ngEqdO/SXbl+qvYgv1GGdNXsncT0+gyR7nGkXpSWlT jTeahiNvGYqtHl0LR3LaRdHTKg8Fa1E5BesIBD5KiasZkkzZoz4YAQhZiPJnFJqM I49tKi3mGG4Bwgm/mRwVQE7EwW4HWkizJCrC+ZIlWO8iSQ+Jalg3CFK3JfXg2ZTY PiiCgFxbUBxAYT4Ux61KbBiyMFF7EpQJ3jmVNGSlOWiFbrZcsTzPtWtItHqR7O41 4S7aiylniOob+iq5CpBirTMbpT+/vAw3gyBKyo2aN8NNC7nXb6JVAEesmjHlTFNC Z0Pm/v+QaKu5Vw8UkKSLqp7HM8jRu53NBpb+TQDGxLgfw4XuiYjgSHAeOxH0glj1 WcQZcSE898CaBiJU4m96MsFJtzFst+/CDKZ6cP/h9VsDyA+pAdZ7kY1CAp5ON0Vr ZjpOWB4iYc4BejxyZzGffJGk71Md6lCkhp1Txs8kXN9vWdOtag5U1vAze1TjsBC8 kU9+1UHhu6dM/BJ0NrcsSgQOFyWZzMsaABT4vJ/XtDpedAuDoc2t2UTZIlWaHr2P PpCAXGaeHN0lsbOTb10+anXryFqU1g6zNPnk9g/Bq0AoscGzPAYBTbTdg95P3Snf eMSf/a/wimbTSlgHkw3R =dI1q -END PGP SIGNATURE- -- Johan
Re: Staging the 1.11.0 website updates
Julian Foad wrote: > Can someone add to staging : remove WIP banner from release notes? (I > can get to it later but AFK now.) Done in r1845177. -- - Julian
Re: Staging the 1.11.0 website updates
Can someone add to staging : remove WIP banner from release notes? (I can get to it later but AFK now.) - Julian
Re: svn commit: r1845151 - in /subversion/site/staging: doap.rdf docs/release-notes/release-history.html download.html index.html news.html
julianf...@apache.org wrote on Mon, Oct 29, 2018 at 17:29:37 -: > In 'staging': stage the changes for 1.11.0 release. That's a good use of staging ☺ It does make me wonder if we need staging/ to have a different robots.txt than publish/, though? We've linked to staging/ on this mailing list so the spiders must have found it.
Re: svn commit: r1845013 - /subversion/trunk/subversion/tests/cmdline/basic_tests.py
Good morning Brane,
br...@apache.org wrote on Sun, 28 Oct 2018 10:40 +:
> Add XFAIL tests for issue #4530.
>
> +++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Sun Oct 28
> 10:40:04 2018
> @@ -3050,6 +3050,33 @@ def peg_rev_on_non_existent_wc_path(sbox
> +def do_move_with_at_signs(sbox, src, dst, dst_cmdline):
> + sbox.build()
> +
> + expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
> + expected_status.tweak(src, status='D ', moved_to=dst)
> + expected_status.add({dst: Item(status='A ', copied='+',
> + moved_from=src, wc_rev='-')})
> +
> + sbox.simple_move(src, dst_cmdline)
> + svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
> +
> +@Issue(4530)
> +@XFail()
> +def move_to_target_with_leading_at_sign(sbox):
> + "rename to dir/@file"
> +
> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon')
> +
> +
> +@Issue(4530)
> +@XFail()
> +def move_to_target_with_leading_and_trailing_at_sign(sbox):
> + "rename to dir/@file@"
> +
> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon@')
I'm not actually sure these are supposed to work as these tests expect:
supporting A/@upsilon as the first argument like this would prevent us
from adding "foo@upsilon" as a command-line syntax for '"foo" at peg
revision svn_opt_revision_working' in a future minor release. I think
this line of thought applies to the second argument as well.
It would probably be good to review the list of intended changes before
time is spent implementing them. Is there such a list? (I don't recall
seeing one on the issue.)
I can see a number of options:
- All PATH arguments (and all TARGET formal arguments when the actual
argument isn't a URI) are parsed for peg revisions, no exceptions.
- Ditto, except when the argument denotes an unversioned un-disk path
(like the targets of «svn add», the file passed as argument to the
«--targets» option, and so on).
- Ditto, plus something that excludes the second target argument to
«svn move». (I'm not sure about how exactly to do this because the syntax
is so ambiguous: «svn move file1 file2», «svn move baz foo/bar» where
«foo/» exists and bar doesn't, and «svn move baz foo/bar» where
«foo/bar/» is a directory, are all valid.)
I'm sure there are more options, but my point is, let's agree on what
the CLI documentation on escaping should say in 1.12.
Cheers,
Daniel
Staging the 1.11.0 website updates
I just pushed the website updates for 1.11.0 to staging: https://subversion-staging.apache.org/ https://subversion-staging.apache.org/news https://subversion-staging.apache.org/download https://subversion-staging.apache.org/docs/release-notes/release-history.html https://subversion-staging.apache.org/doap (is not HTML) Please review if you have a mind to. I hope to be able to publish tomorrow 30th Oct (that's the date I've put in the website and CHANGES file in the tarball) which would be possible if we can get 3 signatures today, or at the very latest the day after (which would be close enough for the discrepancy not to matter, and would still achieve our stated October release date target). -- - Julian
Re: Subversion 1.11.0 up for testing/signing
On Mon, Oct 29, 2018 at 02:53:59PM +, Julian Foad wrote: > The 1.11.0 release artifacts are now available for testing/signing. > Please get the tarballs from > https://dist.apache.org/repos/dist/dev/subversion > and add your signatures there. > > Differences from 1.11.0-rc2 should be limited to the following, so checking > the diff for these should be sufficient -- no need to re-test. > - version number > - CHANGES file > - translations I have verified the differences to -rc2 and they look fine. Signatures below. subversion-1.11.0.tar.bz2 -BEGIN PGP SIGNATURE- iQEcBAABAgAGBQJb1zUpAAoJEE99uqmaWblz9pYIAIwvMAL+cVPLywSTvTYIqFmz 9qgcE/nej/vzFIv/Bk6+8r4OIdlYCHEpk7hN17BupqeYXfBpAiiKc9CatoP15W1w kcvaySY3VMB3Q/vBJjqakLdwcKElgZmCc62LmBVmuAzzo3ufOKn/YoCrNSB3KzQd vq96kgzvfWUtnfqnmuJHA02WKTWaV4vBy8sL5NzBm79iBZeAPFDGeD0wuszQuUkd m+ng6W0KUDMxjcRjQVwP/mXfwVaJr0pFGglhc63rVaa7bCZmsM7V7MMXlcvfGe9m +5J3A3DmXUQgqSaODvRNaFsF51mhG4g+r2feQGJz4bpyv8Xeaf2Fd1gHdaECp0Q= =7Uwa -END PGP SIGNATURE- subversion-1.11.0.tar.gz -BEGIN PGP SIGNATURE- iQEcBAABAgAGBQJb1zUiAAoJEE99uqmaWblzdBkIAI8RsXWnRjL702WaH5z1okOi d1O++7BJ2hL1ghczUjonshGdc8OSkiaWAV9xA9HtKyl0yUMP0vOqV0TY83b3my0E FuboXjOUiCKi9JMPemIdZRoX2pER2a/bec+46/rSJxopL1zEcbNdjOS0rL2Yw07Y 0wcc8QprEz5kjkm8aRKG6LurycK+eLOEQMnFW+4mjntMxkIV6zYu/rSM1cOIgbS/ HVEwIQCHpoJMtHhraHv8HUIikgERhIpPZqI0AftyOFgdTtYK3Viq4nA0WHblfD+I vKWtokErNdugtimTTT/LvraO4kEZWRS4etKH6VnkefQrRF8jbrXRF95907YnUKc= =DTCJ -END PGP SIGNATURE-
Issue with 'svn diff' when file name starts with @
(I tried searching the issue tracker but it was extremely unresponsive; apologies if this is already in there (which seems likely)) Initially encountered on Windows (most recent TortoiseSVN). Reproduced on Linux (Debian testing, subversion-1.10.3-1). The help for svn diff suggests that it supports peg revisions (TARGET[@REV]). However, this does not seem to be the case. In fact, while most (all?) other svn commands will treat 'foo' and 'foo@' as equivalent file names, svn diff does not; 'svn diff foo@' (when only 'foo' exists) will result in E155010 (the node was not found). (This seems part of SVN-3231.) In addition, there is a significant bug in handling of file names starting with @; a diff for them can only be acquired by running 'svn diff' in their directory and using './'. (I don't immediately see a ticket for this case.) Reproduction case: create a new repo containing 2 files: foo/@bar and foo/xy@zzy Check out the repository (messages below will assume it is in /wc): $ cd /wc $ svn diff foo/@bar svn: E155010: The node '/wc/foo@bar' was not found. $ svn diff foo/@bar@ svn: E155010: The node '/wc/foo@bar@' was not found. $ svn diff foo/xy@zzy $ svn diff foo/xy@zzy@ svn: E155010: The node '/wc/foo/xy@zzy@' was not found. $ cd foo $ svn diff @bar svn: E125001: '@bar' is just a peg revision. Maybe try '@bar@' instead? $ svn diff @bar@ svn: E155010: The node '/wc/foo/@bar' was not found. $ svn diff ./@bar $ svn diff ./@bar@ svn: E155010: The node '/wc/foo/@bar@' was not found. So: * the normal 'append-@-to-avoid-peg-revision' syntax does not work (but in some cases the error message suggests using it) * files containing @ anywhere but at the start work fine * files with @ at the start seem to lose the directory separator in front of it, breaking diff operation * exception: using ./ as path while in the file's directory (so luckily this means I have a workaround)
[PATCH] Coredump when javahl SVNClient::diff() called with diff-cmd set
[[[ Prevent some coredumps when using JavaHL SVNClient::diff() SVNClient::diff() discards output to stderr by setting stderr parameters to NULL in some calls Some of the called code can dereference the NULL in some situations. One such situation is if the user has set a diff-cmd value in their settings file. The documented interface to relevant routines does not say that stderr can be set to NULL in this way. Observed in the 1.10.3 client * subversion/bindings/javahl/native/SVNClient.cpp (SVNClient::diff): Replace NULL stderr in calls to (deprecated) svn_client_diff_peg6() and svn_client_diff6() with svn_stream_empty objects. ]]] Index: subversion/bindings/javahl/native/SVNClient.cpp === --- subversion/bindings/javahl/native/SVNClient.cpp (revision 1845130) +++ subversion/bindings/javahl/native/SVNClient.cpp (working copy) @@ -1055,7 +1055,8 @@ void SVNClient::diff(const char *target1, Revision options.useGitDiffFormat(), SVN_APR_LOCALE_CHARSET, outputStream.getStream(subPool), - NULL /* error file */, + // discard stderr + svn_stream_empty(subPool.getPool()), changelists.array(subPool), ctx, subPool.getPool()), @@ -1084,7 +1085,8 @@ void SVNClient::diff(const char *target1, Revision options.useGitDiffFormat(), SVN_APR_LOCALE_CHARSET, outputStream.getStream(subPool), - NULL /* error stream */, + // discard stderr + svn_stream_empty(subPool.getPool()), changelists.array(subPool), ctx, subPool.getPool()),
Subversion 1.11.0 up for testing/signing
The 1.11.0 release artifacts are now available for testing/signing. Please get the tarballs from https://dist.apache.org/repos/dist/dev/subversion and add your signatures there. Differences from 1.11.0-rc2 should be limited to the following, so checking the diff for these should be sufficient -- no need to re-test. - version number - CHANGES file - translations It would be really nice if we can get this done ASAP so we can achieve our planned October release month. Thanks! -- - Julian
Re: svn commit: r1845066 - /subversion/trunk/notes/dump-load-format.txt
This actually depends on what APR uses on different machines. We (as Subversion) fully depend on APR to generate the UUID, and different platforms may (and will) use different techniques. APR has its own fallback completely depending on date/time+randomness when a platform doesn't provide its own UUID api. On Windows XP and later, the UuidCreate() function implementing this function for us doesn't encode the MAC. On older (now unsupported) windows versions it did encode it. I'm not sure if this kind of implementation detail belongs in the documentation. Bert On Sun, Oct 28, 2018 at 10:14 PM, wrote: > Author: esr > Date: Sun Oct 28 21:14:03 2018 > New Revision: 1845066 > > URL: http://svn.apache.org/viewvc?rev=1845066=rev > Log: > * notes/dump-load-format.txt: More about uuid format > > Modified: > subversion/trunk/notes/dump-load-format.txt > > Modified: subversion/trunk/notes/dump-load-format.txt > URL: http://svn.apache.org/viewvc/subversion/trunk/notes/dump- > load-format.txt?rev=1845066=1845065=1845066=diff > > == > --- subversion/trunk/notes/dump-load-format.txt (original) > +++ subversion/trunk/notes/dump-load-format.txt Sun Oct 28 21:14:03 2018 > @@ -72,6 +72,10 @@ UUID: > where the is the UUID of the originating repository. > An example UUID is "7bf7a5ef-cabf-0310-b7d4-93df341afa7e". > > +As generated by Subversion, these UUIDs are "Version 1", incorporating > +the MAC of the originating machine. The presentation is in RFC4122 > +form without the "urn:" or "uuid:" prefixes. > + > Revision records > > A Revision record has three headers and is usually followed by a > > >
Re: [PATCH] Proof of concept of the better-pristines (LZ4 + storing small pristines as BLOBs) (Was: Re: svn commit: r1843076)
On 29.10.2018 14:27, Bert Huijben wrote: > On Windows' NTFS implementation very small files (probably something > like < 256 bytes, but this is not documented/strictly stable) are > stored in the directory table and so don't use 'a whole cluster'. Right — and this is exactly the kind of optimisation we're aiming for by putting small(ish) pristine blobs directly into the working copy database. -- Brane
Re: First update achieved.
Eric S. Raymond wrote: (re. Subversion can also store the credentials encrypted on disk ...) > What's he recommended procedure under Linux? In ~/.subversion/config : [auth] password-stores = gpg-agent,gnome-keyring,kwallet or just list the single one that you want to use. And of course ensure one of those daemons is running. - Julian
Re: [PATCH] Proof of concept of the better-pristines (LZ4 + storing small pristines as BLOBs) (Was: Re: svn commit: r1843076)
On Windows' NTFS implementation very small files (probably something like < 256 bytes, but this is not documented/strictly stable) are stored in the directory table and so don't use 'a whole cluster'. Nice work on all the research! Bert On Tue, Oct 23, 2018 at 6:12 PM, Branko Čibej wrote: > On 22.10.2018 22:14, Evgeny Kotkov wrote: > > Branko Čibej writes: > > > >> Still missing is a mechanism for the libsvn_wc (and possibly > >> libsvn_client) to determine the capabilities of the working copy at > >> runtime (this will be needed for deciding whether to use compressed > >> pristines). > > FWIW, I tried the idea of using LZ4 to compress the pristines and > storing small > > pristines as blobs in the `PRISTINE` table. I was particularly > interested in > > how such change would affect the performance and what kind of obstacles > > would have to be dealt with. > > Nice! I did some simpler tests by compressing exported trees, but this > is definitely better. > > > In the attachment you will find a more or less functional implementation > of > > this idea that might be useful to some extent. The patch is a proof of > > concept: it doesn't include the WC compatibility bits and most certainly > > doesn't have everything necessary in place. But in the meanwhile, I > think > > that is might give a good approximation of what can be expected from the > > approach. > > > > The patch applies to the `better-pristines` branch. > > > > A couple of observations: > > > > - As expected, the combined size of the pristines is halved when the > data > >itself is compressible, thus making the working copy 25% smaller. > > Yes, that was my observation as well. In fact, though, storing small > BLOBs in the database itself should have even better effects, since the > space on disk actually used by a file is rounded up to the nearest > cluster size, but SQLite's blocks are typically much smaller than that. > > > > - A variety of the callers currently access the pristine contents by > reading > >the corresponding files. That doesn't work in case of compressed > pristines > >or pristines stored as BLOBs. > > > >I think that ideally we would want to use streams as much as > possible, and > >only spill the uncompressed pristine contents to temporary files when > we > >need to pass them to external tools, etc.; and that temporary files > need > >to be backed by a work queue to avoid leaving them in place in case > of an > >application crash. > > Yes and yes. Keeping those temporary spilled files on disk could turn > out to be a problem, finding a reasonable time to delete them without > having to run cleanup will be rather important, I think. > > > >The patch does that kind of plumbing to some extent, but that part of > the > >work is not complete. The starting point is around wc_db_pristine.c: > >svn_wc__db_pristine_get_path(). > > > > - Using BLOBs to store the pristine contents didn't have a measurable > impact > >on the speed of the WC operations such as checkout in my experiments > on > >Windows. These experiments were not comprehensive, and also I didn't > run > >the tests on *nix. > > I wouldn't expect much change in performance but would expect better use > of the disk, as explained above. > > > - There's also the deprecated svn_wc_get_pristine_copy_path() public > API that > >would require plumbing to maintain compatibility; the patch performs > it by > >spilling the pristine contents result into a temporary file whose > lifetime > >is attached to the `result_pool`. > > Ack; that's one reasonable definition of "lifetime." But I suspect that > any users of that function expect the pristine file to survive at least > to the next WC cleanup. > > > (I probably won't be able to continue the work on this patch in the > nearby > > future; posting this in case it might be useful.) > > Thanks, it definitely is useful! > > -- Brane > >
Re: Issue tracker "priority" field meaning [was: [jira] [Updated] (SVN-4555) Centralized user level pristine storage]
On 29.10.2018 14:00, Julian Foad wrote: > Branko Čibej wrote: >> On 28.10.2018 14:40, Julian Foad wrote: [ https://issues.apache.org/jira/browse/SVN-4555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Priority: Trivial (was: Major) >>> Brane: why this change? 'Major' (which is the default/usual) priority >>> seemed right to me. >> There are a number of problems with this idea, [...] >> IMO there are much, much more important things we should be doing [...] >> Therefore I'd say that centralised pristine storage is far down the list >> of features we'd like to add. > I don't disagree. It would be good to copy those observations into the issue. Will do. > As for priority, the label "trivial" suggests no real impact and we tend to > use that for items such as "information in a FAQ entry is outdated" > (https://issues.apache.org/jira/browse/SVN-4682). [...] > The old issue tracker used values "P1 ... "P5" for the "Priority" field, and > described it as "level of importance ... to help determine the priority ... > P1 - Most important ... P5 - Least important": > http://subversion.tigris.org/scdocs/ddIssues_EnterModify.html#priority Indeed, "modern" issue trackers have been ignoring the difference between the "priority" and "severity" of an issue and conflate them in some form of "priority" field. I really don't like that, because it blurs the difference between the "political" and "engineering" aspects of an issue. > I would submit that since the majority originate in the old issue tracker, > this meaning is most prevalent. That's possible. In this particular case I did not worry about how the reporter of the issue feels, since this was a breadcrumb we'd created for ourselves. But if we use the Priority field to record the effort or severity, and I feel that "Major" is not correct, calling it "Critical" or "Blocker" is hardly helpful — this particular feature has almost zero impact on the day-to-day usage of subversion (disk is cheap, remember?) but can have a rather important effect in edge cases. What to do? > So maybe we should move all the existing "priority" field values to a field > named "importance", with the sole exception of any you have deliberately set > to actually mean priority. Ah, if we call it "importance" then "Trivial" is correct for this feature, IMO. -- Brane
Issue tracker "priority" field meaning [was: [jira] [Updated] (SVN-4555) Centralized user level pristine storage]
Branko Čibej wrote: > On 28.10.2018 14:40, Julian Foad wrote: > >> [ > >> https://issues.apache.org/jira/browse/SVN-4555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel > >> ] > >> Priority: Trivial (was: Major) > > Brane: why this change? 'Major' (which is the default/usual) priority > > seemed right to me. > > There are a number of problems with this idea, [...] > IMO there are much, much more important things we should be doing [...] > Therefore I'd say that centralised pristine storage is far down the list > of features we'd like to add. I don't disagree. It would be good to copy those observations into the issue. As for priority, the label "trivial" suggests no real impact and we tend to use that for items such as "information in a FAQ entry is outdated" (https://issues.apache.org/jira/browse/SVN-4682). The reporter of such an issue is likely to understand that we feel the issue is of no importance, or that we have completely missed the point, and I want us to maintain a good relationship with our community peers (users). It seems to me that we really don't mean "priority" literally in our use of the issue tracker: we hardly ever record a decision about the order in which issues will be addressed. Rather we usually mean something like "severity" or "effort", probably conflating those two. In fact, the terms like "trivial" and "major" inherently describe something like a severity or effort rather than a priority. The old issue tracker used values "P1 ... "P5" for the "Priority" field, and described it as "level of importance ... to help determine the priority ... P1 - Most important ... P5 - Least important": http://subversion.tigris.org/scdocs/ddIssues_EnterModify.html#priority I would submit that since the majority originate in the old issue tracker, this meaning is most prevalent. So maybe we should move all the existing "priority" field values to a field named "importance", with the sole exception of any you have deliberately set to actually mean priority. -- - Julian
