Thanks for the comments and fixes. A few comments below - rest LGTM!
Den tis 15 sep. 2026 kl 01:34 skrev Pavel Lyalyakin < [email protected]>: > On Sun, 13 Sept 2026 at 23:09, Daniel Sahlberg < > [email protected]> wrote: > >> Den fre 11 sep. 2026 kl 23:52 skrev Pavel Lyalyakin via dev < >> [email protected]>: >> >>> Hello, >>> >>> I'm working on the INSTALL file, aiming to bring it up to date with the >>> current state of Subversion and resolve the issues that accumulated over >>> the decades since the file was introduced (and it's been there since the >>> very birth of SVN). This is still a work in progress, but I believe the >>> current state in trunk is a substantial improvement over the INSTALL file >>> in the 1.15.x branch. Any feedback would be greatly appreciated! >>> >>> The pending INSTALL changes in trunk have been nominated for backport to >>> 1.15.x in the "INSTALL: Corrections and cleanups" group. However, it >>> appears that documentation changes typically don't need a formal voting >>> process and don't restart the soak period when backported[1]. If that's >>> true then I think that it makes sense to confirm that changes to INSTALL >>> fall into the documentation category and let future INSTALL changes be >>> backported without voting. >>> >>> Please let me know what you think. Thank you! >>> >>> [1]: >>> https://subversion.apache.org/docs/community-guide/releasing.html#release-stabilization-backportable-changes >>> >>> -- >>> With best regards, >>> Pavel Lyalyakin >>> VisualSVN Team >>> >> >> Hi Pavel! >> >> Thanks for the hard work improving our documentation! >> > > Daniel, thank you for looking at the document and the changes! > > >> While you are correct that a documentation change doesn't need a vote, I >> think it would be valuable if we can have one or two pairs of eyes look >> over these changes. I will try to do as much as I can but time is a bit >> limited. >> > > I couldn't agree more about the value of peer review! But intuitively I > feel that the backport nomination process involving STATUS isn't the best > approach in such scenarios. I genuinely feel that something is off with the > INSTALL commits accumulating in the STATUS file. > Yes, the STATUS file doesn't work in this case. I think it would be great of someone native English speaker could also take a quick glance at the file if something doesn't make sense from a grammatical (I had to look up the spelling!) point of view. > > >> Some feedback, not only based on the parts you've touch but other things >> that now stand out from the stellar updates: >> >> >> In A.2 Building from a Working Copy: >> >> [[[ >> You can discard the directory created by the tarball; you're >> about to build the latest, greatest Subversion client. This is >> the procedure Subversion developers use. >> ]]] >> >> Originally (see for example r849967) there were text about how to >> bootstrap your environment by building the svn binary from a release >> tarball and only later checking out a working copy and building. I believe >> this paragraph doesn't make sense in the current context and could be >> removed completely. >> > > Yep, that indeed looks like a leftover from some earlier step-by-step > guidance that first required the reader to build SVN from a tarball and > then proceed to building SVN from the latest source / a working copy. And I > agree this doesn't make sense with the current layout of the document. > Fixed in r1938205[1]. > > [[[ >> Start the process by running "autogen.sh": >> >> $ sh ./autogen.sh >> >> _This script will make sure you have all the necessary components >> available to build Subversion. If any are missing, you will be >> told where to get them from._ (See the 'Dependency Overview' in >> section I.) >> ]]] >> >> For me "necessary components" is APR and friends. As far as I can tell, >> autogen doesn't perform this check and for things it does check (for >> example autoconf) it only reports an error. I believe this was a thing >> before r840381. >> > > It appears that this description of autogen.sh was introduced in > r840624[2], and it seems to me that it was inaccurate even then, because it > described the pre-r840381 behavior. Prior to r840381[3], the script checked > for APR and neon and provided a hint if they were missing. This changed in > r840381, so it seems that the description was already out of date when it > was added. > > To fix this, I decided to remove the description in r1938214[4]. > > BTW, I believe that with the autoconf-based build system, ./configure > handles dependency checking, not autogen.sh. E.g., when APR isn't found: > > [[[ > configure: Apache Portable Runtime (APR) library configuration > checking for APR... no > configure: WARNING: APR not found > The Apache Portable Runtime (APR) library cannot be found. > Please install APR on this system and configure Subversion > with the appropriate --with-apr option. > > You probably need to do something similar with the Apache > Portable Runtime Utility (APRUTIL) library and then configure > Subversion with both the --with-apr and --with-apr-util options. > > configure: error: no suitable APR found > ]]] > > >> >> Under A.3 Building In a Separate Build Directory: >> >> [[[ >> $ chmod +x autogen.sh >> $ ./autogen.sh >> ]]] >> >> I think the chmod is not required. autogen.sh is already svn:executable >> in any recent working copy (since r845231) and should be in all tar based >> release tarballs. >> >> For consistency, we might want to use sh ./autogen.sh here as well >> (compare above) or use only ./autogen.sh above. >> > > Let me think about this a little bit more. But sure, the formatting of > examples has to be consistent through the document and I'm in favor of just > ./autogen.sh. > For me, the unneccessary $ chmod was the main thing here. Consistency in formatting matters but less than a command that is effectively no-op. > > >> >> Under D.2 Running the test suite under the autoconf/make build system: >> >> Should we mention check-swig-[py, pl, rb]? I think they are important to >> run but of course they depend on building the bindings. >> > > I haven't yet dived into building and testing the bindings, but I think > that the topic needs to be covered in ./subversion/bindings/swig/INSTALL. A > link to the file will do I guess. > I think that's fine. I've sinced realised that swig/INSTALL mention the checks. > > >> Under IV. DEPENDENCIES IN DETAIL: >> >> [[[ >> ...so if you are in a real hurry to get building, you can skip >> straight to section II. >> ]]] >> >> Reword the "skip straight to" part since we are now below section II? >> > > Since 'Dependencies in Detail' moved to the bottom of the document, I > think that these phrases are simply no longer necessary. Removed in > r1938215[5]. > > >> >> Under 10. Python (https://www.python.org/) (OPTIONAL): >> >> [[[ >> ...However, Support for Python >> 2.7 is being phased out. >> ]]] >> >> Lowercase "s"? >> > > Fixed in r1938203[6] where I updated the Python version requirements. > > >> >> Whole section 17. py3c (OPTIONAL) >> >> If I understand correctly, py3c is only required for the Python bindings. >> There is a separate document (subversion/bindings/swig/INSTALL, also >> referenced in INSTALL) for the bindings which also mention py3c. Swig >> details are only mentioned in the separate document. Does it make sense to >> remove py3c from INSTALL since it is covered elsewhere? Maybe just say >> something about "additional dependencies may be required for the bindings" >> in section IV? For reference we don't say anything about JDK in INSTALL, >> this is only mentioned in the javahl README. >> > > I haven't yet looked into the SWIG bindings topic. I believe that this is > a big topic just by itself and isolating it in a dedicated document > ./subversion/bindings/swig/INSTALL was a really good idea. IMHO moving the > details on building and testing the bindings away from the main INSTALL > file is a good decision. So I'm +1 on removing py3c from the main INSTALL > document. > +1 > > Thank you! > > [1]: https://svn.apache.org/viewvc/?revision=1938205&view=revision > [2]: https://svn.apache.org/viewvc/?revision=840624&view=revision > [3]: https://svn.apache.org/viewvc/?revision=840381&view=revision > [4]: https://svn.apache.org/viewvc/?revision=1938214&view=revision > [5]: https://svn.apache.org/viewvc/?revision=1938215&view=revision > [6]: https://svn.apache.org/viewvc/?revision=1938203&view=revision > > -- > With best regards, > Pavel Lyalyakin > VisualSVN Team >
