| Message ID | pull.1306.v2.git.1659109272.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mingw: include the Python parts in the build | expand |
Hi, On Fri, 29 Jul 2022, Johannes Schindelin via GitGitGadget wrote: > Range-diff vs v1: > > -: ----------- > 1: 5d9b087625a windows: include the Python bits when building Git for Windows > -: ----------- > 2: 019fb837d68 mingw: remove unneeded `NO_GETTEXT` directive > 1: a5739b9cce8 ! 3: 7dc0a1a9aa8 mingw: include the Python parts in the build > @@ Metadata > Author: Johannes Schindelin <Johannes.Schindelin@gmx.de> > > ## Commit message ## > - mingw: include the Python parts in the build > + mingw: remove unneeded `NO_CURL` directive > > - While Git for Windows does not _ship_ Python (in order to save on > - bandwidth), MSYS2 provides very fine Python interpreters that users can > - easily take advantage of, by using Git for Windows within its SDK. > + In df5218b4c30 (config.mak.uname: support MSys2, 2016-01-13), we > + introduced support for building Git for Windows in the then-brand new > + Git for Windows v2.x build environment that was based off of MSYS2. > + > + To do that, we split the non-msysGit part (that targeted MSys1) in two, > + and instead of sharing the `NO_CURL = YesPlease` setting with MSys1, we > + overrode it for MSYS2 with the empty value because we very much want to > + build Git for Windows with libcurl. > + > + But that was unnecessary: we never set that variable beforehand, > + therefore there is no need to override it. > + > + Let's just remove that unnecessary line. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > ## config.mak.uname ## > @@ config.mak.uname: else > + HAVE_LIBCHARSET_H = YesPlease > + USE_GETTEXT_SCHEME = fallthrough > USE_LIBPCRE = YesPlease > - NO_CURL = > +- NO_CURL = > USE_NED_ALLOCATOR = YesPlease > -+ NO_PYTHON = > ifeq (/mingw64,$(subst 32,64,$(prefix))) > # Move system config into top-level /etc/ > - ETC_GITCONFIG = ../etc/gitconfig Oh, that's funny. This is actually the first time I personally see `range-diff` matching up a wrong patch pair (because it really looks for the minimal diff between the diffs). It is of course nonsense to match up the original patch with the `NO_CURL` patch. Ciao, Dscho
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Fri, 29 Jul 2022, Johannes Schindelin via GitGitGadget wrote: > >> Range-diff vs v1: >> >> -: ----------- > 1: 5d9b087625a windows: include the Python bits when building Git for Windows >> -: ----------- > 2: 019fb837d68 mingw: remove unneeded `NO_GETTEXT` directive >> 1: a5739b9cce8 ! 3: 7dc0a1a9aa8 mingw: include the Python parts in the build > ... > Oh, that's funny. This is actually the first time I personally see > `range-diff` matching up a wrong patch pair (because it really looks for > the minimal diff between the diffs). It is of course nonsense to match up > the original patch with the `NO_CURL` patch. It would depend on the creation-factor number, I suspect. To me, it does not seem to match anything at all, but with an unreasonably high number like 9999, I see 1 corresponds to the old one, with the other two follow-up patch as new. As the maintainer, I mostly use range-diff to compare two iterations of a single topic, and not "compare 'seen' from 24 hours ago with 'seen' I just rebuilt, so that I can match up everything in an uncontrolled mess", so the optimum factor number would be different for my usecase from the one used for general use (which is documented to be 60). The "maintainer" use case compares two iterations that are known and expected to have corresponding patches (and no corresponding one means either dropped or added), and come to think of it, the use case for submitter to run "format-patch --range-diff" shares exactly the same expectation. It is very different from "pick corresponding patches from two piles of many unrelated topics" use case, in which "range-diff" proper can be used. Perhaps the default used for "format-patch" should become different and set a lot higher than the default for "range-diff" proper?
Hi Junio, On Fri, 29 Jul 2022, Junio C Hamano wrote: > Perhaps the default used for "format-patch" should become different > and set a lot higher than the default for "range-diff" proper? Good idea. However, this will require careful research about the best value to use. An idea would be to use the lore archive to extract patch series iterations that have been sent to the Git mailing list, then use a variation of https://github.com/dscho/git/tree/range-diff-from-mbox to compare them using `range-diff` with multiple creation factors to determine the bounds within which the optimal value lies. Sadly a bit too involved a project for me to take on right now. Ciao, Dscho
I've actually had a variation of the patch to include th Python bits in Git for Windows's build ever since February 2015. Changes since v1: * Instead of setting and then overriding NO_PYTHON, it is now only defined in the relevant parts of the Windows-specific section of config.mak.uname. * As Junio pointed out, there is an unneeded empty definition of NO_GETTEXT; Let's remove it. * The same holds for NO_CURL: No need to define it to the empty value. Johannes Schindelin (3): windows: include the Python bits when building Git for Windows mingw: remove unneeded `NO_GETTEXT` directive mingw: remove unneeded `NO_CURL` directive config.mak.uname | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) base-commit: 23b219f8e3f2adfb0441e135f0a880e6124f766c Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1306%2Fdscho%2Fmsys2-python-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1306/dscho/msys2-python-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1306 Range-diff vs v1: -: ----------- > 1: 5d9b087625a windows: include the Python bits when building Git for Windows -: ----------- > 2: 019fb837d68 mingw: remove unneeded `NO_GETTEXT` directive 1: a5739b9cce8 ! 3: 7dc0a1a9aa8 mingw: include the Python parts in the build @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@gmx.de> ## Commit message ## - mingw: include the Python parts in the build + mingw: remove unneeded `NO_CURL` directive - While Git for Windows does not _ship_ Python (in order to save on - bandwidth), MSYS2 provides very fine Python interpreters that users can - easily take advantage of, by using Git for Windows within its SDK. + In df5218b4c30 (config.mak.uname: support MSys2, 2016-01-13), we + introduced support for building Git for Windows in the then-brand new + Git for Windows v2.x build environment that was based off of MSYS2. + + To do that, we split the non-msysGit part (that targeted MSys1) in two, + and instead of sharing the `NO_CURL = YesPlease` setting with MSys1, we + overrode it for MSYS2 with the empty value because we very much want to + build Git for Windows with libcurl. + + But that was unnecessary: we never set that variable beforehand, + therefore there is no need to override it. + + Let's just remove that unnecessary line. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## config.mak.uname ## @@ config.mak.uname: else + HAVE_LIBCHARSET_H = YesPlease + USE_GETTEXT_SCHEME = fallthrough USE_LIBPCRE = YesPlease - NO_CURL = +- NO_CURL = USE_NED_ALLOCATOR = YesPlease -+ NO_PYTHON = ifeq (/mingw64,$(subst 32,64,$(prefix))) # Move system config into top-level /etc/ - ETC_GITCONFIG = ../etc/gitconfig