git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/2] Feature: New Variable git-p4.binary

From
Luke Diamand <luke@diamand.org>
Date
Nov 14, 2019, 09:53 UTC
Message-ID
<CAE5ih7-_ZESWAGAiXL4vT-P0aVYtrOAHn6dbhsfTmmeKVkSUWA@mail.gmail.com>
In-Reply-To
<xmqq5zjn9oaz.fsf@gitster-ct.c.googlers.com>
On Thu, 14 Nov 2019 at 02:36, Junio C Hamano <gitster@pobox.com> wrote:
Show 37 quoted lines
>
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Issue: Using git-p4.py on Windows does not resolve properly to the p4.exe
> > binary in all instances.
> >
> > Two new code features are added to resolve the p4 executable location:
> >
> >  1. A new variable, git-p4.binary, has been added that takes precedence over
> >     the default p4 executable name. If this git option is set and the
> >     path.exists() passes for this file it will be used as executable for the
> >     system.popen calls.
> >
> >
> >  2. If the new variable git-p4.binary is not set, the program checks if the
> >     operating system is Windows. If it is, the executable is changed to
> >     'p4.exe'. All other operating systems
> >     (those that do not report 'Windows' in the platform.system() call)
> >     continue to use the current executable of 'p4'.
>
> I do not use Windows nor git-p4, but the above two changes make
> sense to me at the design level.  One thing that needs to be updated
> is the configuration variable, though.  It is more in line with the
> other parts of the system to name this "git-p4.program", because
> binary-ness does not matter much to you, as long as the named thing
> works correctly as "p4" program replacement.
>
> Seeing "gpg.program" is used in a similar way, it also is tempting
> to call it "p4.program", but no other parts of Git other than
> "git-p4" uses P4, and the "git-p4." prefix is to collect variables
> related to "git-p4" together, so calling it "p4.program" may not be
> a good idea---it would hurt discoverability.  "git-p4.p4program"
> may be OK, if we anticipate that git-p4 may need to use (and its
> users need to specify paths to) external programs other than "p4",
> but it probably is overkill.
>
> Thanks.
There's some trailing whitespace in 98bae, can we get that tidied up?
Otherwise, modulo Junio's comments, it looks good.

I agree that "git-p4.binary" might be harder to discover ("Oh, I assumed that enabled binary mode!"). p4program should be fine.

Previous: Junio C HamanoNext: Ben Keene
Message 5 of 17 in “Feature: New Variable git-p4.binary”
  1. 0/2 Feature: New Variable git-p4.binaryBen Keene via GitGitGadget, Nov 13, 2019
  2. 2/2 Added general variable git-p4.binary and added a default for windows of 'P4.EXE'Ben Keene via GitGitGadget, Nov 13, 2019
  3. 1/2 Cast byte strings to unicode strings in python3Ben Keene via GitGitGadget, Nov 13, 2019
  4. Junio C HamanoNov 14, 2019
  5. Luke DiamandNov 14, 2019
  6. Ben KeeneNov 14, 2019
  7. Luke DiamandNov 15, 2019
  8. 0/3 Feature: New Variable git-p4.p4programBen Keene via GitGitGadget, Nov 15, 2019
  9. 1/3 Cast byte strings to unicode strings in python3Ben Keene via GitGitGadget, Nov 15, 2019
  10. Junio C HamanoNov 16, 2019
  11. Junio C HamanoNov 16, 2019
  12. 3/3 Changed the name of the parameter from git-p4.binary to git-p4.p4programBen Keene via GitGitGadget, Nov 15, 2019
  13. 2/3 Added general variable git-p4.binary and added a default for windows of 'P4.EXE'Ben Keene via GitGitGadget, Nov 15, 2019
  14. Junio C HamanoNov 16, 2019
  15. Junio C HamanoNov 16, 2019
  16. Junio C HamanoNov 18, 2019
  17. Ben KeeneDec 3, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.