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

Re: [PATCH] diff: add support for reading files literally with --no-index

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Jan 4, 2019, 02:57 UTC
Message-ID
<20190104025724.GG423984@genre.crustytoothpaste.net>
In-Reply-To
<20190104021855.GB97188@google.com>
On Thu, Jan 03, 2019 at 06:18:55PM -0800, Jonathan Nieder wrote:
Show 22 quoted lines
> I may be a minority in this opinion, but I had trouble understanding
> what --literally would do from its name.  I suspect we can come up
> with a better name.
> 
> Unfortunately, I'm terrible at coming up with names. :-P
> 
> --dereference would be a good name when it comes to symlinks, but
> it's not a good name for reading what is on the other side of a pipe.
> On the plus side, it matches "diff" and "cp"'s name for the "follow
> symbolic links" option.
> 
> --plain captures the desire a little better --- we want a plain
> read(2) from the file instead of trying to be smart and look at
> whether it is e.g. a block device.  But in the context of "diff", that
> would seem more like an option that affects the output.
> 
> What would you think of
> 
>  - always reading from fifos instead of describing them, since I've
>    never encountered a use case where people want the latter
> 
>  - --dereference to control whether to follow symlinks

This is actually surprisingly difficult. The reason I implemented this only for no-index mode is because there are actually several places we can stat a file in the diff code, and implementing a --dereference option that catches all of those cases and getting the option passed down to them is non-trivial.

Since my primary environment is on Linux, I need to implement functional --dereference semantics before implementing the FIFO functionality to know if it works in my use case.

I agree that would be a nicer approach overall, so let me see if I can take another stab at it in a way that isn't excessively complex. It would also make the name less controversial, since "--dereference" is the obvious choice for dereferencing symlinks.

Show 24 quoted lines
> [...]
> > --- a/diff-no-index.c
> > +++ b/diff-no-index.c
> > @@ -75,7 +75,25 @@ static int populate_from_stdin(struct diff_filespec *s)
> >  	return 0;
> >  }
> >  
> > -static struct diff_filespec *noindex_filespec(const char *name, int mode)
> > +static int populate_literally(struct diff_filespec *s)
> > +{
> > +	struct strbuf buf = STRBUF_INIT;
> > +	size_t size = 0;
> > +	int fd = xopen(s->path, O_RDONLY);
> > +
> > +	if (strbuf_read(&buf, fd, 0) < 0)
> > +		return error_errno("error while reading from '%s'", s->path);
> > +
> > +	s->should_munmap = 0;
> > +	s->data = strbuf_detach(&buf, &size);
> > +	s->size = size;
> > +	s->should_free = 1;
> > +	s->read_literally = 1;
> 
> Oh!  --read-literally works perfectly for me as a name. :)

If I can't manage to split this out into two pieces, I'll pick that as the name for the reroll.

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Previous: Jonathan NiederNext: Junio C Hamano
Message 16 of 18 in “diff: add support for reading files literally with --no-index”
  1. diff: add support for reading files literally with --no-indexbrian m. carlson, Dec 20, 2018
  2. Jeff KingDec 20, 2018
  3. brian m. carlsonDec 21, 2018
  4. Duy NguyenDec 20, 2018
  5. Jeff KingDec 20, 2018
  6. Duy NguyenDec 20, 2018
  7. Jeff KingDec 20, 2018
  8. Duy NguyenDec 20, 2018
  9. Ævar Arnfjörð BjarmasonDec 20, 2018
  10. brian m. carlsonDec 20, 2018
  11. Johannes SchindelinDec 21, 2018
  12. brian m. carlsonDec 21, 2018
  13. Junio C HamanoJan 2, 2019
  14. brian m. carlsonJan 4, 2019
  15. Jonathan NiederJan 4, 2019
  16. brian m. carlsonJan 4, 2019
  17. Junio C HamanoJan 4, 2019
  18. brian m. carlsonJan 5, 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.