Re: svn commit: r1845378 - /subversion/trunk/CHANGES
On 31.10.2018 21:08, Daniel Shahaf wrote: > br...@apache.org wrote on Wed, 31 Oct 2018 19:42 +: >> +++ subversion/trunk/CHANGES Wed Oct 31 19:42:05 2018 >> @@ -26,6 +26,7 @@ https://svn.apache.org/repos/asf/subvers >> +* Storing passowrds in plain text on disk is disabled by default >> (r1845377) > Typo "passwords" Thanks, will fix. > Other issue: In the three months centered on the 1.12.0 release we'll be > supporting 1.9, 1.10, 1.11, 1.12, and trunk in parallel, so I think we > should reduce the CHANGES management effort. To take this commit as an > example, while brane added a line to the 1.12 CHANGES, the RM's of 1.9, > 1.10, 1.11 will have to copy it manually. That adds up. Nope, they will not; this change will not be backported. -- Brane
Re: svn commit: r1845378 - /subversion/trunk/CHANGES
br...@apache.org wrote on Wed, 31 Oct 2018 19:42 +: > +++ subversion/trunk/CHANGES Wed Oct 31 19:42:05 2018 > @@ -26,6 +26,7 @@ https://svn.apache.org/repos/asf/subvers > +* Storing passowrds in plain text on disk is disabled by default > (r1845377) Typo "passwords" Other issue: In the three months centered on the 1.12.0 release we'll be supporting 1.9, 1.10, 1.11, 1.12, and trunk in parallel, so I think we should reduce the CHANGES management effort. To take this commit as an example, while brane added a line to the 1.12 CHANGES, the RM's of 1.9, 1.10, 1.11 will have to copy it manually. That adds up. The first idea that comes to mind is to put the CHANGES info in the log message (as I advised rpluem@ yesterday to do), whence 'release.py write-changelog' can pick it automatically. Other approaches are possible, of course; I don't mind which one is chosen¹, but I do think we should do plan ahead and make sure we can generate the CHANGES sections for stable lines with as little effort as possible. Cheers, Daniel ¹ Though write-changelog has already been implemented, and is really very sensible, so I don't see any reason to switch away from it.
Re: svn commit: r1845312 - /subversion/trunk/tools/dist/backport.pl
On 2018/10/31 06:54:35, Daniel Shahaf wrote: > Good morning Ruediger, > > rpl...@apache.org wrote on Wed, 31 Oct 2018 06:45 +: > > Adjust grep syntax to be able to run it with Perl 5.16.3. > > > > * tools/dist/backport.pl > > (vote) Adjust grep syntax to be able to run it with Perl 5.16.3. > > > > [D:other] backport.pl: Make it run with Perl 5.16.3. > > Thanks for committing the fix. > > In CHANGES, "developer-visible" means "visible to API consumers". > backport.pl, however, is not visible to API consumers; it's only used > by developers working on Subversion itself. As such, this changelog > line, although syntactically correct, is wrong. The correct line would > be [c:skip]. Ahh, sorry. Makes sense. I misunderstood the comments in release.py regarding D: > > I'll go ahead and make it so. Thanks Regards Rüdiger
Re: SVN version numbering
On Wed, Oct 31, 2018 at 6:14 AM Thomas Singer wrote: > Hi all, > > OK, we are now at SVN 1.11 because you have agreed to release often with > only a few changes. What does the prefix "1." mean - will there be some > "2." or "3." in the future? If not, then it is redundant. For time-based > releases, wouldn't it be more useful to use the year, e.g. the next one > could be SVN 19 or SVN 19.04? > > Cheers, > Tom > Please don't. Some Linux distros do their version numbering this way and I can never understand what that means. Much better for version numbers to denote API compatibility as Subversion does.
Re: [PATCH] Suppress conflict resolver in dry-run merge
On Wed, Oct 31, 2018 at 09:46:23AM +, Jonathan Guy wrote:
> Hi Stephan
> Thanks for the reply.
> I did see code section you refer to however the svn_client__resolve_conflicts
> will only be called if you have ctx->conflict_func2.
> That function (AFAIK) is only installed if an “accept” option is supplied as
> the code below
>
>
> /* Install a legacy conflict handler if the --accept option was given.
>* Else, svn_client_merge5() may abort the merge in an undesirable way.
>* See the docstring at conflict_func_merge_cmd() for details */
> if (opt_state->accept_which != svn_cl__accept_unspecified)
> {
> struct conflict_func_merge_cmd_baton *b = apr_pcalloc(pool, sizeof(*b));
>
> b->accept_which = opt_state->accept_which;
> SVN_ERR(svn_dirent_get_absolute(>path_prefix, "", pool));
> b->conflict_stats = conflict_stats;
>
> ctx->conflict_func2 = conflict_func_merge_cmd;
> ctx->conflict_baton2 = b;
> }
>
>
> The resolver that runs post run_merge is however run regardless if there is
> an accept argument or not.
> If no accept option is given the user is ultimately prompted to resolve the
> conflict (dry-run or no dry-run).
>
>
> /* If we are in interactive mode and either the user gave no --accept
>* option or the option did not apply, then prompt. */
> if (option_id == svn_client_conflict_option_unspecified)
> {
> svn_boolean_t resolved = FALSE;
> svn_boolean_t postponed = FALSE;
> svn_boolean_t printed_description = FALSE;
> svn_error_t *err;
>
> *quit = FALSE;
>
> while (!resolved && !postponed && !*quit)
> {
> err = resolve_conflict_interactively(, ,
>
>
> Sorry I’m no trying to be argumentative I just need to clear on this one
> aspect.
> Thanks for any help.
Can you provide an example? I cannot reproduce the problem like this:
$ svn merge ^/trunk
--- Merging r2 through r5 into '.':
C alpha
Aalpha2
--- Recording mergeinfo for merge of r2 through r5 into '.':
U .
Summary of conflicts:
Tree conflicts: 1
Searching tree conflict details for 'alpha' in repository:
Checking r4... done
Tree conflict on 'alpha':
File merged from
'^/trunk/alpha@1'
to
'^/trunk/alpha@5'
was moved to '^/trunk/alpha2' by stsp in r4.
A file which differs from the corresponding file on the merge source branch was
found in the working copy.
Applying recommended resolution 'Move and merge':
Calpha2
Tree conflict at 'alpha' marked as resolved.
Merge conflict discovered in file 'alpha2'.
Select: (p) Postpone, (df) Show diff, (e) Edit file, (m) Merge,
(s) Show all options: q
Summary of conflicts:
Text conflicts: 1 remaining (and 0 already resolved)
Tree conflicts: 0 remaining (and 1 already resolved)
$ sv revert
$ pwd
/tmp/svn-sandbox/branch
$ svn revert -R .
Reverted '.'
Reverted 'alpha'
Reverted 'alpha2'
$ svn merge ^/trunk --dry-run
--- Merging r2 through r5 into '.':
C alpha
Aalpha2
Summary of conflicts:
Tree conflicts: 1
$
Re: Disabling plain-text password storage by defult
On 31.10.2018 13:01, Julian Foad wrote: > Stefan Sperling wrote: >> On Wed, Oct 31, 2018 at 02:49:57AM +0100, Branko Čibej wrote: >>> Given that we support a number of secure credentials stores, I >> propose >>> that, starting with 1.14.0 LTS, we disable the on-disk plain-text >>> password store by default. >> Why not start with 1.12.0? > +1 to doing it. +1 to starting with 1.12. That works for me, too. I'd somehow thought an LTS release was a big enough deal for such a change, but on consideration, you're right; changes like this should be made in regular releases, that's what they're for. I'll commit an appropriate variant of the patch. -- Brane
Re: Disabling plain-text password storage by defult
Stefan Sperling wrote: >On Wed, Oct 31, 2018 at 02:49:57AM +0100, Branko Čibej wrote: >> Given that we support a number of secure credentials stores, I >propose >> that, starting with 1.14.0 LTS, we disable the on-disk plain-text >> password store by default. > >Why not start with 1.12.0? +1 to doing it. +1 to starting with 1.12. - Julian
Re: SVN version numbering
On 31.10.2018 11:13, Thomas Singer wrote: > Hi all, > > OK, we are now at SVN 1.11 because you have agreed to release often > with only a few changes. What does the prefix "1." mean - will there > be some "2." or "3." in the future? If not, then it is redundant. For > time-based releases, wouldn't it be more useful to use the year, e.g. > the next one could be SVN 19 or SVN 19.04? https://subversion.apache.org/docs/community-guide/releasing.html#release-compat -- Brane
Re: SVN version numbering
What does the prefix "1." mean Major versions are usually used to denote API versions. It is expected that on at least some level, all minor revisions of version 1 are somewhat compatible to each other, bar new features introduced in later minor revisions. Conversely, there is no expectation that SVN 2.X is in any way compatible with SVN 1.X. This way, a software is future-proof in the sense that if someone would invent a complete new way SVN could work (faster etc.), it can still be called SVN instead of FOOBAR. Example: GRUB and GRUB 2. If not, then it is redundant. For time-based releases, wouldn't it be more useful to use the year, e.g. the next one could be SVN 19 or SVN 19.04? It is not redundant, as demonstrated. Let's not abandon something that has been proven useful in the wild just because it seems a good thing to do. It isn't. Regards, From: Thomas Singer Sent: Wednesday, October 31, 2018 4:13 PM To: dev@subversion.apache.org Subject: SVN version numbering Hi all, OK, we are now at SVN 1.11 because you have agreed to release often with only a few changes. What does the prefix "1." mean - will there be some "2." or "3." in the future? If not, then it is redundant. For time-based releases, wouldn't it be more useful to use the year, e.g. the next one could be SVN 19 or SVN 19.04? Cheers, Tom
SVN version numbering
Hi all, OK, we are now at SVN 1.11 because you have agreed to release often with only a few changes. What does the prefix "1." mean - will there be some "2." or "3." in the future? If not, then it is redundant. For time-based releases, wouldn't it be more useful to use the year, e.g. the next one could be SVN 19 or SVN 19.04? Cheers, Tom
Re: [PATCH] Suppress conflict resolver in dry-run merge
Hi Stephan
Thanks for the reply.
I did see code section you refer to however the svn_client__resolve_conflicts
will only be called if you have ctx->conflict_func2.
That function (AFAIK) is only installed if an “accept” option is supplied as
the code below
/* Install a legacy conflict handler if the --accept option was given.
* Else, svn_client_merge5() may abort the merge in an undesirable way.
* See the docstring at conflict_func_merge_cmd() for details */
if (opt_state->accept_which != svn_cl__accept_unspecified)
{
struct conflict_func_merge_cmd_baton *b = apr_pcalloc(pool, sizeof(*b));
b->accept_which = opt_state->accept_which;
SVN_ERR(svn_dirent_get_absolute(>path_prefix, "", pool));
b->conflict_stats = conflict_stats;
ctx->conflict_func2 = conflict_func_merge_cmd;
ctx->conflict_baton2 = b;
}
The resolver that runs post run_merge is however run regardless if there is an
accept argument or not.
If no accept option is given the user is ultimately prompted to resolve the
conflict (dry-run or no dry-run).
/* If we are in interactive mode and either the user gave no --accept
* option or the option did not apply, then prompt. */
if (option_id == svn_client_conflict_option_unspecified)
{
svn_boolean_t resolved = FALSE;
svn_boolean_t postponed = FALSE;
svn_boolean_t printed_description = FALSE;
svn_error_t *err;
*quit = FALSE;
while (!resolved && !postponed && !*quit)
{
err = resolve_conflict_interactively(, ,
Sorry I’m no trying to be argumentative I just need to clear on this one aspect.
Thanks for any help.
Regards
Jonathan
> On 31 Oct 2018, at 08:32, Stefan Sperling wrote:
>
> On Tue, Oct 30, 2018 at 06:39:49PM +, Jonathan Guy wrote:
>> [[[
>> *subversion/svn/merge-cmd.c
>> (svn_cl__merge): Suppress the interactive conflict resolver
>> if a merge has been performed with the dry-run option.
>> ]]]
>
> Hi Jonathan,
>
> Thank your for the patch. I am afraid I misled you earlier by suggesting
> that you send a patch for the issue. This is in fact not a problem in
> the current implementation because the dry_run flag gets passed down
> into libsvn_client, where it takes part in the decision about running
> the conflict resolver.
>
> Specifically, this part of the do_merge() function in the file
> subversion/libsvn_client/merge.c checks the flag:
>
> /* Give the conflict resolver callback the opportunity to
> * resolve any conflicts that were raised. If it resolves all
> * of them, go around again to merge the next sub-range (if any). */
> if (conflicted_range_report && ctx->conflict_func2 && ! dry_run)
>{
> svn_boolean_t conflicts_remain;
>
> SVN_ERR(svn_client__resolve_conflicts(
>_remain, merge_cmd_baton.conflicted_paths,
>ctx, iterpool));
> if (conflicts_remain)
>break;
>
> So your patch is redundant and we don't need to apply it.
> I should have tested the current behaviour before recommending that
> you send a patch.
>
> Regardless, thank you for your contribution!
>
> Stefan
Re: [PATCH] Suppress conflict resolver in dry-run merge
On Tue, Oct 30, 2018 at 06:39:49PM +, Jonathan Guy wrote:
> [[[
> *subversion/svn/merge-cmd.c
> (svn_cl__merge): Suppress the interactive conflict resolver
> if a merge has been performed with the dry-run option.
> ]]]
Hi Jonathan,
Thank your for the patch. I am afraid I misled you earlier by suggesting
that you send a patch for the issue. This is in fact not a problem in
the current implementation because the dry_run flag gets passed down
into libsvn_client, where it takes part in the decision about running
the conflict resolver.
Specifically, this part of the do_merge() function in the file
subversion/libsvn_client/merge.c checks the flag:
/* Give the conflict resolver callback the opportunity to
* resolve any conflicts that were raised. If it resolves all
* of them, go around again to merge the next sub-range (if any). */
if (conflicted_range_report && ctx->conflict_func2 && ! dry_run)
{
svn_boolean_t conflicts_remain;
SVN_ERR(svn_client__resolve_conflicts(
_remain, merge_cmd_baton.conflicted_paths,
ctx, iterpool));
if (conflicts_remain)
break;
So your patch is redundant and we don't need to apply it.
I should have tested the current behaviour before recommending that
you send a patch.
Regardless, thank you for your contribution!
Stefan
Re: Disabling plain-text password storage by defult
On Wed, Oct 31, 2018 at 02:49:57AM +0100, Branko Čibej wrote: > Given that we support a number of secure credentials stores, I propose > that, starting with 1.14.0 LTS, we disable the on-disk plain-text > password store by default. Why not start with 1.12.0?
Re: svn commit: r1845312 - /subversion/trunk/tools/dist/backport.pl
Good morning Ruediger, rpl...@apache.org wrote on Wed, 31 Oct 2018 06:45 +: > Adjust grep syntax to be able to run it with Perl 5.16.3. > > * tools/dist/backport.pl > (vote) Adjust grep syntax to be able to run it with Perl 5.16.3. > > [D:other] backport.pl: Make it run with Perl 5.16.3. Thanks for committing the fix. In CHANGES, "developer-visible" means "visible to API consumers". backport.pl, however, is not visible to API consumers; it's only used by developers working on Subversion itself. As such, this changelog line, although syntactically correct, is wrong. The correct line would be [c:skip]. I'll go ahead and make it so. Cheers, Daniel
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 21:11:57, Daniel Shahaf wrote:
> Ruediger Pluem wrote on Tue, Oct 30, 2018 at 19:57:58 -:
> > BTW, I had to apply the following patch to nominate.pl to get it running
> > with perl 5.16.3 on CentOS 7:
> >
> > Index: tools/dist/backport.pl
> > ===
> > --- tools/dist/backport.pl (revision 1845203)
> > +++ tools/dist/backport.pl (working copy)
> > @@ -791,8 +791,8 @@
> >
> > # Add to state votes that aren't '+0' or 'edit'
> > $state->{$_->{digest}}++ for grep
> > - ($_->{approval} or $_->{vote} =~
> > /^(-1|-0|[+]1)$/),
> > - @votesarray;
> > + (($_->{approval} or $_->{vote} =~
> > /^(-1|-0|[+]1)$/),
> > + @votesarray);
> >}
> > }
> >
> > I am happy to commit this if this is fine with later versions of perl which
> > I have not at hand for testing.
>
> Works for me with and without the change, Perl 5.24.1 on Debian stretch.
> +1 to commit.
>
> Consider using «grep +(foo), @bar» or «grep { foo } @bar» instead of
> «grep ((foo), bar»: the three alternatives are equivalent, so it's just
> a question of which one is more readable/idiomatic.
Used the grep +(foo), @bar syntax and committed in r1845312
Regards
Rüdiger
Re: svn commit: r1845204 - in /subversion/trunk/subversion: mod_authz_svn/mod_authz_svn.c mod_dav_svn/mod_dav_svn.c
On 2018/10/30 21:03:53, Daniel Shahaf wrote: > Ruediger Pluem wrote on Tue, Oct 30, 2018 at 20:00:01 -: > > > > > > On 2018/10/30 18:09:42, Daniel Shahaf wrote: > > > Branko Čibej wrote on Tue, 30 Oct 2018 10:22 +0100: > > > > Should I add the above to the commit log? Or should I add an entry to > > CHANGES directly? > > I can do either way whatever you prefer. > > In the log message please; release.py write-changelog will pick it from there > when we roll 1.12.0-alpha1. > Adjusted svn:log property of r1845204. Regards Rüdiger
