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

Re: [PATCH 8/9] Use the early config machinery to expand aliases

From
BWBrandon Williams <bmwill@google.com>
Date
Jun 8, 2017, 14:51 UTC
Message-ID
<20170608145100.GJ110638@google.com>
In-Reply-To
<alpine.DEB.2.21.1.1706081223130.171564@virtualbox>
On 06/08, Johannes Schindelin wrote:
Show 50 quoted lines
> Hi Brandon,
> 
> On Wed, 7 Jun 2017, Brandon Williams wrote:
> 
> > I like seeing chunks of old code being deleted :D
> 
> Me, too. In particular as hacky code as this one. It caused quite a couple
> of gray hairs here.
> 
> > On 06/07, Johannes Schindelin wrote:
> > 
> > > @@ -245,36 +201,37 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> > >  
> > >  static int handle_alias(int *argcp, const char ***argv)
> > >  {
> > > +	struct strbuf worktree_dir = STRBUF_INIT;
> > >  	int envchanged = 0, ret = 0, saved_errno = errno;
> > >  	int count, option_count;
> > >  	const char **new_argv;
> > >  	const char *alias_command;
> > >  	char *alias_string;
> > > -	int unused_nongit;
> > > -
> > > -	save_env_before_alias();
> > > -	setup_git_directory_gently(&unused_nongit);
> > >  
> > >  	alias_command = (*argv)[0];
> > > -	alias_string = alias_lookup(alias_command, NULL);
> > > +	alias_string = alias_lookup(alias_command, &worktree_dir);
> > >  	if (alias_string) {
> > >  		if (alias_string[0] == '!') {
> > >  			struct child_process child = CHILD_PROCESS_INIT;
> > >  
> > > +			if (worktree_dir.len)
> > > +				setup_git_directory();
> > 
> > So if there is a worktree then we run the setup code explicitly.  I
> > assume that is to ensure that all envvars are setup properly before
> > running the alias.  Just interesting to note that the actual value of
> > worktree_dir is never used, its just used essentially as a boolean
> > indicator of there being a worktree/gitdir/repository.  I'm not
> > suggesting to change from what you have here, its just food for thought.
> 
> Indeed.
> 
> That is what I tried to indicate by this paragraph in the commit message:
> 
> 	Rather than just chdir()ing into the indicated directory in case of an
> 	alias expanding to a shell command, we simply set up the .git/ directory
> 	so that e.g. GIT_PREFIX is set as expected.

I was more commenting on "oh that's what he meant by that" for my own understanding. But thanks again for pointing out that you did include a comment in the commit message reflecting that.

-- 
Brandon Williams
Previous: Johannes SchindelinNext: Junio C Hamano
Message 29 of 34 in “Avoid problem where git_dir is set after alias expansion”
  1. 0/9 Avoid problem where git_dir is set after alias expansionJohannes Schindelin, Jun 7, 2017
  2. 1/9 discover_git_directory(): avoid setting invalid git_dirJohannes Schindelin, Jun 7, 2017
  3. Brandon WilliamsJun 7, 2017
  4. Johannes SchindelinJun 8, 2017
  5. 2/9 config: report correct line number upon errorJohannes Schindelin, Jun 7, 2017
  6. Junio C HamanoJun 9, 2017
  7. 3/9 help: use early config when autocorrecting aliasesJohannes Schindelin, Jun 7, 2017
  8. Brandon WilliamsJun 7, 2017
  9. Johannes SchindelinJun 8, 2017
  10. Junio C HamanoJun 9, 2017
  11. 4/9 read_early_config(): optionally return the worktree's top-level directoryJohannes Schindelin, Jun 7, 2017
  12. Brandon WilliamsJun 7, 2017
  13. Johannes SchindelinJun 8, 2017
  14. Brandon WilliamsJun 8, 2017
  15. Johannes SchindelinJun 8, 2017
  16. Brandon WilliamsJun 8, 2017
  17. Johannes SchindelinJun 8, 2017
  18. Brandon WilliamsJun 8, 2017
  19. 7/9 alias_lookup(): optionally return top-level directoryJohannes Schindelin, Jun 7, 2017
  20. 6/9 t7006: demonstrate a problem with aliases in subdirectoriesJohannes Schindelin, Jun 7, 2017
  21. 5/9 t1308: relax the test verifying that empty alias values are disallowedJohannes Schindelin, Jun 7, 2017
  22. Brandon WilliamsJun 7, 2017
  23. Johannes SchindelinJun 8, 2017
  24. Brandon WilliamsJun 8, 2017
  25. Junio C HamanoJun 10, 2017
  26. 8/9 Use the early config machinery to expand aliasesJohannes Schindelin, Jun 7, 2017
  27. Brandon WilliamsJun 7, 2017
  28. Johannes SchindelinJun 8, 2017
  29. Brandon WilliamsJun 8, 2017
  30. Junio C HamanoJun 10, 2017
  31. Johannes SchindelinJun 7, 2017
  32. Brandon WilliamsJun 7, 2017
  33. Johannes SchindelinJun 8, 2017
  34. Brandon WilliamsJun 8, 2017

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.