Re: svn commit: r1911830 - /subversion/branches/1.14.x/STATUS
On Tue, Aug 22, 2023 at 7:08 PM Nathan Hartman wrote: > > On Mon, Aug 21, 2023 at 12:59 PM Daniel Sahlberg > wrote: >> >> I'm at a loss why the backport.pl script doesn't do the backport of the >> approved r1906502. When running the script manually, it reports the >> candidate changes but nothing under Approved. >> >> [[[ >> svnsvn@svn-qavm1:~/src/svn/1.14.x$ ../trunk/tools/dist/backport.pl >> >> >> === Candidate changes: >> >> >> >>> The r1890223 group: >> r1890223, r1890668, r1890673 >> >> Support building on Win64/ARM64. >> >> [...] >> libsvn_client: Pass redirected URL for file externals. >> >> +1: dsahlberg, stsp >> >> Run a merge? [y,l,v,±1,±0,q,e,a, ,N,?] >> >> >> === Veto-blocked changes: >> >> >> === Approved changes: >> svnsvn@svn-qavm1:~/src/svn/1.14.x$ >> ]]] >> >> I'm sure there is some subtle formatting error in the approved change, but I >> can't see it. > > > > I think it needs one more newline between the row of equal signs and " * > r1906502" Added that in r1911850. Cheers, Nathan
Re: svn commit: r1911830 - /subversion/branches/1.14.x/STATUS
On Mon, Aug 21, 2023 at 12:59 PM Daniel Sahlberg < daniel.l.sahlb...@gmail.com> wrote: > I'm at a loss why the backport.pl script doesn't do the backport of the > approved r1906502. When running the script manually, it reports the > candidate changes but nothing under Approved. > > [[[ > svnsvn@svn-qavm1:~/src/svn/1.14.x$ ../trunk/tools/dist/backport.pl > > > === Candidate changes: > > > >>> The r1890223 group: > r1890223, r1890668, r1890673 > > Support building on Win64/ARM64. > > [...] > libsvn_client: Pass redirected URL for file externals. > > +1: dsahlberg, stsp > > Run a merge? [y,l,v,±1,±0,q,e,a, ,N,?] > > > === Veto-blocked changes: > > > === Approved changes: > svnsvn@svn-qavm1:~/src/svn/1.14.x$ > ]]] > > I'm sure there is some subtle formatting error in the approved change, but > I can't see it. > I think it needs one more newline between the row of equal signs and " * r1906502" Nathan
Re: svn commit: r1903267 - /subversion/trunk/Makefile.in
On Sun, Aug 7, 2022 at 6:48 AM wrote:
>
> Author: futatuki
> Date: Sun Aug 7 10:48:25 2022
> New Revision: 1903267
>
> URL: http://svn.apache.org/viewvc?rev=1903267=rev
> Log:
> * Makefile.in
> (fast-clean, clean-swig-py): Remove __pycache__ directory as well as *.pyc
>
> Suggested by: hartmannathan
>
> Modified:
> subversion/trunk/Makefile.in
>
> Modified: subversion/trunk/Makefile.in
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/Makefile.in?rev=1903267=1903266=1903267=diff
> ==
> --- subversion/trunk/Makefile.in (original)
> +++ subversion/trunk/Makefile.in Sun Aug 7 10:48:25 2022
> @@ -457,10 +457,11 @@ fast-clean: doc-clean
> done
> echo $(CLEAN_FILES) | xargs rm -f --
> for d in $(CTYPES_PYTHON_SRC_DIR) $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR) \
> - $(abs_srcdir)/build
> $(top_srcdir)/subversion/tests/cmdline/svntest; \
> + $(abs_srcdir)/build $(top_srcdir)/subversion/tests/cmdline; \
> do \
> test -e $$d || continue; \
> - find $$d -name "*.pyc" -exec rm {} ';'; \
> + find $$d '(' -name "__pycache__" -prune -o -name "*.pyc" ')' \
> + -exec rm -rf {} ';'; \
> done
>
> # clean everything, returning to before './configure' was run.
> @@ -969,7 +970,8 @@ clean-swig-py:
> for d in $(SWIG_PY_SRC_DIR) $(SWIG_PY_DIR); \
> do \
> test -e $$d || continue; \
> - find $$d -name "*.pyc" -exec rm {} ';'; \
> + find $$d '(' -name "__pycache__" -prune -o -name "*.pyc" ')' \
> + -exec rm -rf {} ';'; \
> done
>
> extraclean-swig-py: clean-swig-py
Nice! Thanks for taking care of this. It looks good on code reading.
I'll try to do some builds sometime in the next few days to test this
and other recent changes to the build system.
Cheers,
Nathan
Re: svn commit: r1902582 - /subversion/trunk/tools/dist/release.py
On Thu, Jul 14, 2022 at 3:55 PM Daniel Sahlberg wrote: > > Thanks for the review. I've committed a few changes as r1902722. More below. > > Den ons 13 juli 2022 kl 15:57 skrev Daniel Shahaf : >> >> dsahlb...@apache.org wrote on Fri, Jul 08, 2022 at 20:47:42 -: >> > +++ subversion/trunk/tools/dist/release.py Fri Jul 8 20:47:42 2022 >> > @@ -980,7 +979,12 @@ def roll_tarballs(args): >> > # from a committer's LDAP profile down the road) >> > basename = 'subversion-%s.KEYS' % (str(args.version),) >> > filepath = os.path.join(get_tempdir(args.base_dir), basename) >> > -download_file(KEYS, filepath, None) >> > +# The following code require release.py to be executed within a >> > +# complete wc, not a shallow wc as indicated in HACKING as one >> > option. >> > +# We /could/ download COMMITTERS from /trunk if it doesn't >> > exist... >> >> Well, could you please either change HACKING or download COMMITTERS? >> The code for the latter is basically the tempfile+urlopen mechanics from >> the next hunk of this very diff. > > > I prefer to have less variations in the process to make it easier for new > RMs. I've removed this from HACKING in r1902723 (only on site/staging so far). Thanks for doing that. I had intended to draft some changes for HACKING but it ended up being a busier week than I had anticipated. Nathan
Re: svn commit: r1902590 - /subversion/trunk/tools/client-side/store-plaintext-password.py
On Thu, Jul 14, 2022 at 10:02 AM Daniel Sahlberg wrote: > > Den tors 14 juli 2022 kl 15:52 skrev Daniel Shahaf : >> >> Nathan Hartman wrote on Wed, 13 Jul 2022 15:29 +00:00: >> > On Wed, Jul 13, 2022 at 10:55 AM Daniel Shahaf >> > wrote: >> >> Should the entry link to the zsh script >> >> (https://mail-archives.apache.org/mod_mbox/subversion-dev/202008.mbox/%3C20200816130713.6abca815%40tarpaulin.shahaf.local2%3E) >> >> as well, as an alternative? It might be useful for someone if their >> >> environment doesn't have Python installed or if they find the zsh script >> >> easier to audit. >> > >> > I think it would be useful, and... >> > >> >> (Well, I suppose it might make more sense to copy the script >> >> somewhere than to link to an immutable archives message with that >> >> subject line.) >> > >> > ...the place to put it is probably tools/client-side/ just like the >> > Python script. >> >> Being in tools/ would imply dev@ accepts responsibility for bug reports >> against the zsh script. Is dev@ happy to do that? I'm concerned about >> the bus factor. > > > I was just about to say the same thing (and with no intention to discredit > the zsh version). If it is desirable to list all available realms and let the > user choose interactively, I could add that to the Python script. > > I was also going to add that I think it is better to provide one tool and > make sure that tool is working well instead of having two tools that differ > only in tiny details, since they might bit-rot in different ways over time > and it might be hard for a newcomer to understand the motivation of having > different tools. These are all good points. I admit that zsh is a bit of a mystery to me, as is the script, so I couldn't provide support for it, at least not with my current knowledge. I am impressed that zsh can do so much with so little. It's in the list archives, but as DanielSh points out, is in a thread with a not-so-nice subject. That could be addressed by re-mailing it to dev@ with a new subject, e.g., "Prototype zsh script to store svn password in plaintext" in case anyone ever asks or searches for a non-Python way to do it. We could even link to it from the same FAQ, e.g., "An example of how to store svn plaintext credentials was implemented as a zsh script. It is unsupported by the SVN maintainers but can be found at [link] for pedagogical purposes." Cheers, Nathan
Re: svn commit: r1902590 - /subversion/trunk/tools/client-side/store-plaintext-password.py
On Wed, Jul 13, 2022 at 10:55 AM Daniel Shahaf wrote: > > Nathan Hartman wrote on Wed, 13 Jul 2022 13:43 +00:00: > > On Wed, Jul 13, 2022 at 9:33 AM Daniel Shahaf > > wrote: > > > >> dsahlb...@apache.org wrote on Fri, Jul 08, 2022 at 23:39:14 -: > >> > A new script to store/update a password in the plain text password store > >> > > >> > * tools/client-side/store-plaintext-password.py > >> > As above > >> > > >> > Discussed on dev@: > >> https://lists.apache.org/thread/jfd0f5n2qpgnyc30dst6ycnkphcwf6mm > >> > > >> > Added: > >> > subversion/trunk/tools/client-side/store-plaintext-password.py > >> (with props) > >> > >> Presumably, now that it's been added, we should link it from somewhere > >> to make it discoverable by users? > > > > > > > > Ah yes, it is on my todo list to link to it from the FAQ [1]. :-) > > > > [1] https://subversion.apache.org/faq.html#plaintext-passwords > > Added to staging in r1902704. Hope you don't mind :) Please take it > from here if you have time. Ah, you beat me to it. :-) LGTM. I was about to ask you: > Should the entry link to the zsh script > (https://mail-archives.apache.org/mod_mbox/subversion-dev/202008.mbox/%3C20200816130713.6abca815%40tarpaulin.shahaf.local2%3E) > as well, as an alternative? It might be useful for someone if their > environment doesn't have Python installed or if they find the zsh script > easier to audit. I think it would be useful, and... > (Well, I suppose it might make more sense to copy the script > somewhere than to link to an immutable archives message with that > subject line.) ...the place to put it is probably tools/client-side/ just like the Python script. Cheers, Nathan
Re: svn commit: r1902590 - /subversion/trunk/tools/client-side/store-plaintext-password.py
On Wed, Jul 13, 2022 at 9:33 AM Daniel Shahaf wrote: > dsahlb...@apache.org wrote on Fri, Jul 08, 2022 at 23:39:14 -: > > A new script to store/update a password in the plain text password store > > > > * tools/client-side/store-plaintext-password.py > > As above > > > > Discussed on dev@: > https://lists.apache.org/thread/jfd0f5n2qpgnyc30dst6ycnkphcwf6mm > > > > Added: > > subversion/trunk/tools/client-side/store-plaintext-password.py > (with props) > > Presumably, now that it's been added, we should link it from somewhere > to make it discoverable by users? Ah yes, it is on my todo list to link to it from the FAQ [1]. :-) [1] https://subversion.apache.org/faq.html#plaintext-passwords Cheers, Nathan
Re: svn commit: r1899714 - /subversion/trunk/tools/buildbot/master/README
On Sun, Apr 10, 2022 at 3:15 PM Daniel Sahlberg wrote: > Den sön 10 apr. 2022 kl 21:11 skrev : > >> Author: dsahlberg >> Date: Sun Apr 10 19:11:45 2022 >> New Revision: 1899714 >> >> URL: http://svn.apache.org/viewvc?rev=1899714=rev >> Log: >> Update url to buildbot config repo >> >> * tools/buildbot/master/README: >> Switch to buildbot2, buildbot doesn't exist anymore >> >> Modified: >> subversion/trunk/tools/buildbot/master/README >> >> Modified: subversion/trunk/tools/buildbot/master/README >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/tools/buildbot/master/README?rev=1899714=1899713=1899714=diff >> >> == >> --- subversion/trunk/tools/buildbot/master/README (original) >> +++ subversion/trunk/tools/buildbot/master/README Sun Apr 10 19:11:45 2022 >> @@ -4,4 +4,4 @@ This was announced per this email: >> >> https://mail-archives.apache.org/mod_mbox/subversion-dev/201005.mbox/%3caanlktilvspswjhlljvpkpgvai2-jqygqlqcn1sjgo...@mail.gmail.com%3E >> >> The new BuildBot Master configuration is maintained here: >> - >> https://svn.apache.org/repos/infra/infrastructure/buildbot/aegis/buildmaster/master1/ >> +https://svn.apache.org/repos/infra/infrastructure/buildbot2/projects/ >> >> >> > I think we could also remove the link to a decade-old e-mail, but I'm open > to opinions. > > Yes, especially since some links in the email are broken now. Cheers, Nathan
Re: svn commit: r1899431 - /subversion/trunk/tools/dist/README.backport
On Thu, Mar 31, 2022 at 10:01 AM Daniel Sahlberg wrote: > > Den tors 31 mars 2022 kl 15:24 skrev : >> >> Author: hartmannathan >> Date: Thu Mar 31 13:24:23 2022 >> New Revision: 1899431 >> >> URL: http://svn.apache.org/viewvc?rev=1899431=rev >> Log: >> * tools/dist/README.backport: Update name of machine to svn-qavm. >> >> Modified: >> subversion/trunk/tools/dist/README.backport >> >> Modified: subversion/trunk/tools/dist/README.backport >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/tools/dist/README.backport?rev=1899431=1899430=1899431=diff >> == >> --- subversion/trunk/tools/dist/README.backport (original) >> +++ subversion/trunk/tools/dist/README.backport Thu Mar 31 13:24:23 2022 >> @@ -19,7 +19,7 @@ The scripts are: >> >> backport.pl: >> oldest script, implements [F1], [F2], and [F3]. As of Feb 2018, used in > > > We could change this to As of March 2022. Ah yes, missed that. :-) >> -production by svn-role (running on svn-qavm3) and by >> svn-backport-conflicts-1.9.x >> +production by svn-role (running on svn-qavm) and by >> svn-backport-conflicts-1.9.x >> (a buildbot job). > > > I'm guessing the svn-backports-conflicts-1.9.x job isn't run anymore and it > should be reflected somehow. > > I'm holding back to changing the date until we've figured out the buildbots. > And I'm holding that back until after 1.14.2. Ok no probs... I'll leave it alone for now. Thanks for reviewing. Nathan
Re: svn commit: r1899311 - /subversion/branches/1.14.x/STATUS
On Thu, Mar 31, 2022 at 9:09 AM Nathan Hartman wrote: > My bad. Hopefully r1899430 fixes it. > > How do I manually run backport.pl? Let me rephrase that question: How do I manually trigger it so we don't have to wait for the cron job? Cheers, Nathan
Re: svn commit: r1899311 - /subversion/branches/1.14.x/STATUS
On Thu, Mar 31, 2022 at 6:58 AM Mark Phippard wrote: > > Could someone perhaps manually merge this? > > We need the backports working to finish this process and get the release out. > > > On Thu, Mar 31, 2022 at 2:31 AM Daniel Sahlberg > wrote: > > > > Hi, > > > > The backport below is stuck. backport.pl reports: > > > > [[[ > > Warning summary > > === > > > > 1.14.x-r1881534-no-crlf (Fix issue #4864 "build/ac-macros/macosx.m4: > > workaround AC_RUN_IFELSE"): Revisions 'r1881534' nominated but not included > > in branch > > ]]] > > > > /Daniel My bad. Hopefully r1899430 fixes it. How do I manually run backport.pl? Cheers, Nathan
Re: svn commit: r1899276 - /subversion/site/publish/upcoming.part.html
On Mon, Mar 28, 2022 at 3:55 AM Daniel Sahlberg
wrote:
> This commit doesn't look correct.
>
> I executed the generate-upcoming-changes-log.sh manually yesterday and it
> created r1899244, removing a lot of log entries belonging to 1.14.1. This
> commit (which was executed by cron) restores them.
>
> The crontab entry is:
> [[[
> # Puppet Name: Update our upcoming changes list
> SVN=svn
> 15 4 * * * chronic ~/src/svn/site/tools/generate-upcoming-changes-log.sh
> ]]]
>
> The script begins with a comment
> [[[
> # This should be run from the root of a branches/1.{9,10,11}.x working
> copy.
> ]]]
>
> I suppose the crontab command should be changed to:
> cd ~/src/svn/1.14.x && chronic
> ~/src/svn/site/tools/generate-upcoming-changes-log.sh
>
> However that will create an "upcoming" only for 1.14 and not for 1.10. Any
> thoughts
>
We don't really have a place on the release notes page for "upcoming" for
two different major lines. For now it probably makes sense to show it for
1.14.x only, but when 1.15 is released we'll have to decide whether to show
"upcoming" for just 1.15 or separately for 1.14 and 1.15.
Regarding things disappearing and reappearing, some time ago I noticed that
if an error occurs in the script, such as it can't reach the repo, then it
fails to generate any text but proceeds to commit unconditionally. I don't
remember if that was ever fixed.
Nathan
Re: svn commit: r1896877 - /subversion/trunk/subversion/svnadmin/svnadmin.c
On Mon, Jan 10, 2022 at 6:44 AM wrote: > > Author: stsp > Date: Mon Jan 10 11:44:46 2022 > New Revision: 1896877 > > URL: http://svn.apache.org/viewvc?rev=1896877=rev > Log: > Fix misleading -r option documentation for some svnadmin subcommands. > > The commands delrevprop, lstxns, rev-size, and setrevprop accept only > a single revision number. However, the -r option help text implied > that a revision range was accepted. Correct the -r option help text > of aforementioned commands. The log message contains an error: the corrected commands are: delrevprop, rev-size, setlog, setrevprop The help text of 'svnadmin lstxns' is correct and is unchanged. Cheers, Nathan
Re: svn commit: r1896898 - /subversion/branches/1.14.x/STATUS
On Mon, Jan 10, 2022 at 4:51 PM Daniel Sahlberg wrote: > > Den mån 10 jan. 2022 kl 22:45 skrev : >> >> Author: dsahlberg >> Date: Mon Jan 10 21:45:27 2022 >> New Revision: 1896898 >> >> URL: http://svn.apache.org/viewvc?rev=1896898=rev >> Log: >> * STATUS: Vote for r1896877 (snip) > As far as I understand it, this change require three (or two, for a non-LTS) > votes, even though this is only a documentation fix, right? Yes that's correct. It's in core code and a LTS release... I added the 3rd vote, approving, in r1896905. Thanks, Nathan
Re: svn commit: r1896485 - /subversion/site/staging/download.html
On Thu, Jan 6, 2022 at 1:17 AM Nathan Hartman wrote: > > On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg > wrote: > > > > I think we should now merge staging => publish, everything looks good to > > me. Do you want to check and do the honors? > > Sure. I will check it one more time in the morning and then do it... Done in r1896755 Cheers, Nathan
Re: svn commit: r1896485 - /subversion/site/staging/download.html
On Wed, Jan 5, 2022 at 1:03 PM Daniel Sahlberg wrote: > > Den ons 5 jan. 2022 kl 16:22 skrev Nathan Hartman : >> So, I think it might be a good idea to revert this change and let >> Infra manage how they supply those arrays; if it seems as though >> they're providing an incorrect listing, it should be fixed on their >> end because that will affect all projects, not just Subversion. > > > Done, in r1896725. Thanks! >> The removal of "You may consult the complete list of mirrors" link >> should *not* be reverted. That page is now a 404. > > > That was a separate commit (for easy reverting :-) ). > > I think we should now merge staging => publish, everything looks good to me. > Do you want to check and do the honors? Sure. I will check it one more time in the morning and then do it... Cheers, Nathan
Re: svn commit: r1896485 - /subversion/site/staging/download.html
On Wed, Jan 5, 2022 at 7:45 AM Daniel Sahlberg wrote: > > Den tis 28 dec. 2021 kl 21:14 skrev : >> >> Author: dsahlberg >> Date: Tue Dec 28 20:14:17 2021 >> New Revision: 1896485 >> >> URL: http://svn.apache.org/viewvc?rev=1896485=rev >> Log: >> In site/staging: The ASF download script currently return the same site >> for both "http" and "backup" (after the migration to dlcdn.a.o). >> >> * download.html: Avoid displaying the same URL twice in the >> >> Modified: >> subversion/site/staging/download.html >> >> Modified: subversion/site/staging/download.html >> URL: >> http://svn.apache.org/viewvc/subversion/site/staging/download.html?rev=1896485=1896484=1896485=diff >> == >> --- subversion/site/staging/download.html (original) >> +++ subversion/site/staging/download.html Tue Dec 28 20:14:17 2021 >> @@ -73,9 +73,9 @@ Other mirrors: >>[if-any ftp] >> [for ftp]> selected="selected"[end]>[ftp][end] >>[end] >> - [if-any backup] >> + [if-any backup][is backup http][else][# Only show backup if different >> from http ] >> [for backup]> selected="selected"[end]>[backup] (backup)[end] >> - [end] >> + [end][end] >> >> >> > > > The reasoning behind this change is described in the log message but I'm not > happy about it for a few reasons: > * [http] and [backup] are arrays, so comparing these are probably not a good > idea. I can't come up with anything reasonably simple though. > * The lua version of EZT doesn't seem to support [else] at all. The fact that > it works for me is probably purely accidental. > > I have a strong feeling that the download page will not work in case > different URLs are returned from the "closer.lua" script. Please check if you > are getting different lists in [http] and [backup] from > https://subversion-staging.apache.org/download.cgi?as_json=1 (I'm getting > https://dlcdn.apache.org/ on both of these). In that case check if the > looks reasonable at > https://subversion-staging.apache.org/download.cgi. Also compare with our > published download page at https://subversion.apache.org/download.cgi. > > I'm inclined to revert this change and live with the fact that the download > page can look slightly silly (displaying dlcdn.a.o as both the main and the > backup mirror) until ASF Infra check on closer.lua (or someone else find a > more clever way of writing the EZT comparison code). Interesting. On publish, previously the "Other mirrors" list was showing me dlcdn.a.o for both the main and backup. As of right now, I am getting two options on publish: https://dlcdn.apache.org/ https://downloads.apache.org/ (backup) While on staging, I am only getting https://dlcdn.apache.org/ So, I think it might be a good idea to revert this change and let Infra manage how they supply those arrays; if it seems as though they're providing an incorrect listing, it should be fixed on their end because that will affect all projects, not just Subversion. The removal of "You may consult the complete list of mirrors" link should *not* be reverted. That page is now a 404. Cheers, Nathan
Re: svn commit: r1894491 - /subversion/trunk/INSTALL
On Fri, Oct 22, 2021 at 2:43 PM wrote: > Add a warning regarding the ZLib ASM "optimizations" that are known to be > buggy. > > * INSTALL: In "Building the Latest Source under Windows" (snip) > + Please note that you MUST NOT build ZLib with the included assembler > + optimized code. It is known to be buggy, see for example the discussion > + https://svn.haxx.se/dev/archive-2013-10/0109.shtml. > + This means that you must not define ASMV or ASMINF. Note that the VS > + projects in contrib\visualstudio define these in the Debug > configuration. Thanks for adding this. I do remember reading this somewhere but still can't figure out where. Having it in INSTALL is very helpful. Cheers, Nathan
Re: svn commit: r1891237 - in /subversion/site/staging/docs: ./ api/ api/1.13/ api/1.13/search/ api/1.14/ api/1.14/search/ javahl/ javahl/1.13/ javahl/1.13/org/ javahl/1.13/org/apache/ javahl/1.13/org
On Mon, Jul 5, 2021 at 9:27 PM Daniel Shahaf wrote: > Daniel Sahlberg wrote on Sat, Jul 03, 2021 at 23:45:01 +0200: > > It seems that the API and JavaHL docs for 1.13 and 1.14 are missing. I > have > > generated them from the branches. > > Are there any relevant differences between the branches (@HEAD) and the > latest tags on each branch? > Oh, they probably should have been generated from the latest tags, not the branches. HACKING shows how to generate them from branches; sorry, can't copy/paste the section link because this phone won't cooperate, but search "create or update the versioned documentation snapshots" or some subset of that. Of course, that section assumes it's being done at release time, when the branches would supposedly have the same content as the tags (unless a backport approval race condition occurs). >From my memory, I think all the recent JavaHL work was released and the new C APIs (e.g. the parallelization ones) aren't on any release branch. But I can verify that when I next reach a real computer unless someone beats me to it. Thanks for pointing that out. Nathan
Re: svn commit: r1889114 - /subversion/trunk/subversion/tests/libsvn_subr/task-test.c
On Fri, Apr 23, 2021 at 12:59 AM wrote:
> Modified: subversion/trunk/subversion/tests/libsvn_subr/task-test.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/task-test.c?rev=1889114=1889113=1889114=diff
> ==
> --- subversion/trunk/subversion/tests/libsvn_subr/task-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_subr/task-test.c Fri Apr 23
> 04:59:29 2021
> @@ -124,32 +124,32 @@ counter_func(void **result,
>
>if (value > 1)
> {
> - partial_result = apr_palloc(result_pool, sizeof(partial_result));
> + partial_result = apr_palloc(result_pool, sizeof(*partial_result));
>*partial_result = 1;
>value -= *partial_result;
>
>sub_task_pool = svn_task__create_process_pool(task);
>
> - partial_baton = apr_palloc(sub_task_pool, sizeof(partial_baton));
> + partial_baton = apr_palloc(sub_task_pool, sizeof(*partial_baton));
>*partial_baton = MAX(1, value / 2);
>value -= *partial_baton;
>
> - SVN_ERR(svn_task__add_similar(task, sub_task_pool,
> + SVN_ERR(svn_task__add_similar(task, sub_task_pool,
> partial_result, partial_baton));
> }
>
>if (cancel_func)
> SVN_ERR(cancel_func(cancel_baton));
> -
> +
My editor removed some trailing whitespaces, such as the example
above, and I forgot to un-remove them before committing. :-/
Nathan
Re: svn commit: r1888589 - in /subversion/trunk: build.conf subversion/tests/libsvn_subr/task-test.c
On Wed, Apr 21, 2021 at 11:58 PM Daniel Shahaf wrote: > > + partial_result = apr_palloc(result_pool, sizeof(partial_result)); > > s/sizeof(foo)/sizeof(*foo)/, here and later in the function. > > I'm surprised that no one caught this in post-commit reviews. In fact, > I wonder whether people have reviewed this commit and missed the bug, or > didn't review this commit at all — the latter being a silent failure > mode of the commit-then-review paradigm. (More on this under separate > cover — currently on private@, but may move here later.) (snip) Regarding counter_func(): > I'm not sure I follow what's happening here, and there aren't any > comment breadcrumbs either. I cannot grok this function either. It needs either comments to explain the intent here, or the test should be changed entirely, because I think (from my vague partial understanding) that the test is flawed. However, before jumping to conclusions, I'd like to wait for Stefan^2 to enlighten us. Meanwhile, since these 5 allocations are obviously wrong, I fixed them in r1889114 to prevent them from being forgotten there indefinitely. > test_counting() passes even if I change the > RHS's of both assignments to «*partial_result» to random values. > (test_cancellation() does fail in that case.) It looks to me (again, based on a vague partial understanding) that the test execution will self-adjust for whatever you assign here, as long as you assign a non-negative integer. If you change the RHS to a negative value, the test will (I believe) run for a very long time, and if you assign -1 to both cases or 1 to the first and -1 to the second, I think it will run forever. Two additional (very minor) issues with r1888589: The log message makes no mention of the new file subversion/tests/libsvn_subr/task-test.c, and that file starts with a spurious blank line. (It also had some trailing whitespace on various lines, but I inadvertently removed those in r1889114, much to my chagrin.) Cheers, Nathan
Re: svn commit: r1870062 - /subversion/site/publish/docs/release-notes/1.14.html
On Thu, Nov 21, 2019 at 9:09 PM Daniel Shahaf wrote: > hartmannat...@apache.org wrote on Thu, 21 Nov 2019 04:23 +00:00: > > +The convenience script that downloads Subversion's minimal > > +build-time dependencies, get-deps.sh, has been updated to download > > +py3c. This script is found in the source distribution's root > > +directory. For the full list of Subversion's dependencies, see the > > +INSTALL file in the same directory. > > Suggest to link to the INSTALL file. > > I'm not sure whether "INSTALL" and "get-deps.sh" should be set in , > being filenames. In svnbook they would be, but I'm not sure what our > house style for release notes is. > > Cheers, > > Daniel > I agree and several other files are mentioned that should be links as well. Where should these point? Perhaps to: https://svn.apache.org/viewvc/subversion/trunk/INSTALL?revision=HEAD with a TODO reminding us to change 'trunk' to 'branches/1.14.x'? Thanks, Nathan
