Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH v2 3/5] sha1_file: improve directories comparison method

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:59

Wang Hui [off-list ref] writes:
From: Hui Wang <redacted>

In the past, to check if two directory paths are same, we use memcmp()
to directly compare their path strings, this method can't get an
accurate result if paths include ".." or "." or redundant slash, e.g.
current dir is /, "/a/b/c", "/a/b//c/d/e/../.." and "./a/b/f/../c"
should be the same dir, but current method will identify they are
different.

Now add a global function is_same_directory() to replace the old
memcmp() method, this function will change two input paths to real
path first, then normalized them and compare them.
I do not like this patch _at all_. While it may result in correct result
if you _always_ make it absolute before comparing two entities, if you
will be storing the normalized result after running the comparison anyway,
and if you are comparing against the existing and supposedly already
normalized entities with a new candidate, why would anybody sane would
want to keep paying for the normalization cost at such a low level?

IOW, you are proposing to do:

	given a new candidate;
	for existing entities:
		normalize existing
                normalize candiate
                compare the above two
                if they are equal:
                	ignore
	if no match found
        	add the normalized candidate to the list

Wouldn't it make much more sense to do this:

	given a new candidate;
        normalize it
	for existing entities:
                compare existing and normalized candidate
		there is no point in normalizing the existing one!
                if they are equal:
                	ignore
	if no match found
        	add the normalized candidate to the list
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help