Re: [PATCH 00/24] Kill the_index part3

2018-08-15 Thread Stefan Beller
On Mon, Aug 13, 2018 at 2:24 PM Junio C Hamano  wrote:
>
> Brandon Williams  writes:
>
> > On 08/13, Nguyễn Thái Ngọc Duy wrote:
> >> This is the third part of killing the_index (at least outside
> >> builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This
> >> part is built on top of nd/no-extern.
> >>
> >> This series would actually break 'pu' because builtin/stash.c uses
> >> three functions that are updated here. So we would need something like
> >> the following patch to make it build again.
> >>
> >> I don't know if that adds too much work on Junio. If it does, I guess
> >> I'll hold this off for a while until builtin/stash.c gets merged
> >> because reordering these patches, pushing the patches that break
> >> stash.c away, really takes a lot of work.
> >>
> >> [1] https://public-inbox.org/git/20180616054157.32433-1-pclo...@gmail.com/
> >
> > I went through and found this to be a pleasant read and hopefully others
> > agree with the approach this series took vs what your part 1 did so that
> > we can get this change in.
>
> Yeah, I've only finished my first pass (read: I didn't go through
> the patches with fine toothed comb, nor thought about interactions
> with other topics), but this round was quite a pleasnt read so far.
>

I went through this topic with a finer comb and just found nits,
none of which I would deem a complete show stopper.


Re: [PATCH 00/24] Kill the_index part3

2018-08-13 Thread Junio C Hamano
Brandon Williams  writes:

> On 08/13, Nguyễn Thái Ngọc Duy wrote:
>> This is the third part of killing the_index (at least outside
>> builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This
>> part is built on top of nd/no-extern.
>> 
>> This series would actually break 'pu' because builtin/stash.c uses
>> three functions that are updated here. So we would need something like
>> the following patch to make it build again.
>> 
>> I don't know if that adds too much work on Junio. If it does, I guess
>> I'll hold this off for a while until builtin/stash.c gets merged
>> because reordering these patches, pushing the patches that break
>> stash.c away, really takes a lot of work.
>> 
>> [1] https://public-inbox.org/git/20180616054157.32433-1-pclo...@gmail.com/
>
> I went through and found this to be a pleasant read and hopefully others
> agree with the approach this series took vs what your part 1 did so that
> we can get this change in.

Yeah, I've only finished my first pass (read: I didn't go through
the patches with fine toothed comb, nor thought about interactions
with other topics), but this round was quite a pleasnt read so far.





Re: [PATCH 00/24] Kill the_index part3

2018-08-13 Thread Brandon Williams
On 08/13, Nguyễn Thái Ngọc Duy wrote:
> This is the third part of killing the_index (at least outside
> builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This
> part is built on top of nd/no-extern.
> 
> This series would actually break 'pu' because builtin/stash.c uses
> three functions that are updated here. So we would need something like
> the following patch to make it build again.
> 
> I don't know if that adds too much work on Junio. If it does, I guess
> I'll hold this off for a while until builtin/stash.c gets merged
> because reordering these patches, pushing the patches that break
> stash.c away, really takes a lot of work.
> 
> [1] https://public-inbox.org/git/20180616054157.32433-1-pclo...@gmail.com/

I went through and found this to be a pleasant read and hopefully others
agree with the approach this series took vs what your part 1 did so that
we can get this change in.


-- 
Brandon Williams


[PATCH 00/24] Kill the_index part3

2018-08-13 Thread Nguyễn Thái Ngọc Duy
This is the third part of killing the_index (at least outside
builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This
part is built on top of nd/no-extern.

This series would actually break 'pu' because builtin/stash.c uses
three functions that are updated here. So we would need something like
the following patch to make it build again.

I don't know if that adds too much work on Junio. If it does, I guess
I'll hold this off for a while until builtin/stash.c gets merged
because reordering these patches, pushing the patches that break
stash.c away, really takes a lot of work.

[1] https://public-inbox.org/git/20180616054157.32433-1-pclo...@gmail.com/

diff --git a/builtin/stash.c b/builtin/stash.c
index 74eda822ce..f34edba21f 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -294,7 +294,7 @@ static int apply_patch_from_buf(struct strbuf *patch, int 
cached, int reverse,
const char *patch_path = ".git/stash_patch.patch";
FILE *patch_file;
 
-   if (init_apply_state(, NULL))
+   if (init_apply_state(, the_repository, NULL))
return -1;
 
state.cached = cached;
@@ -873,7 +873,7 @@ static int get_untracked_files(const char **argv, const 
char *prefix,
max_len = fill_directory(, the_repository->index, );
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
-   if (!dir_path_match(ent, , max_len, seen)) {
+   if (!dir_path_match(the_repository->index, ent, , 
max_len, seen)) {
free(ent);
continue;
}
@@ -1299,7 +1299,7 @@ static int do_push_stash(int argc, const char **argv, 
const char *prefix,
 
for (i = 0; i < active_nr; ++i) {
const struct cache_entry *ce = active_cache[i];
-   if (!ce_path_match(ce, , ps_matched))
+   if (!ce_path_match(_index, ce, , ps_matched))
continue;
}
 
Nguyễn Thái Ngọc Duy (24):
  diff.c: move read_index() code back to the caller
  cache-tree: wrap the_index based wrappers with #ifdef
  attr: remove an implicit dependency on the_index
  convert.c: remove an implicit dependency on the_index
  dir.c: remove an implicit dependency on the_index in pathspec code
  preload-index.c: use the right index instead of the_index
  ls-files: correct index argument to get_convert_attr_ascii()
  unpack-trees: remove 'extern' on function declaration
  unpack-trees: add a note about path invalidation
  unpack-trees: don't shadow global var the_index
  unpack-trees: convert clear_ce_flags* to avoid the_index
  unpack-trees: avoid the_index in verify_absent()
  pathspec.c: use the right index instead of the_index
  submodule.c: use the right index instead of the_index
  entry.c: use the right index instead of the_index
  attr: remove index from git_attr_set_direction()
  grep: use the right index instead of the_index
  archive.c: avoid access to the_index
  archive-*.c: use the right repository
  resolve-undo.c: use the right index instead of the_index
  apply.c: pass struct apply_state to more functions
  apply.c: make init_apply_state() take a struct repository
  apply.c: remove implicit dependency on the_index
  blame.c: remove implicit dependency on the_index

 apply.c | 66 +
 apply.h |  4 +++
 archive-tar.c   |  2 +-
 archive-zip.c   |  2 +-
 archive.c   | 47 --
 archive.h   | 16 +++--
 attr.c  | 52 +
 attr.h  | 11 ---
 blame.c | 52 +
 blame.h |  1 +
 builtin/add.c   |  6 ++--
 builtin/am.c|  2 +-
 builtin/apply.c |  2 +-
 builtin/archive.c   |  2 +-
 builtin/blame.c |  1 +
 builtin/cat-file.c  |  2 +-
 builtin/check-attr.c|  6 ++--
 builtin/checkout-index.c|  1 +
 builtin/checkout.c  |  2 +-
 builtin/clean.c |  2 +-
 builtin/commit.c|  2 +-
 builtin/diff-tree.c |  8 +++--
 builtin/grep.c  |  6 ++--
 builtin/ls-files.c  | 17 +-
 builtin/pack-objects.c  |  2 +-
 builtin/rm.c|  2 +-
 builtin/submodule--helper.c |  2 +-
 builtin/update-index.c  |  2 +-
 builtin/upload-archive.c|  3 +-
 cache-tree.c| 12 ---
 cache-tree.h| 17 --
 convert.c   | 41 +--
 convert.h   | 15 ++---
 diff-lib.c  |  4 +--
 diff.c  | 12 +--
 diff.h  |  1 -
 dir.c   | 27 ---
 dir.h   | 16 +
 entry.c |  9