On Tue, Jun 23, 2020 at 02:59:21PM +0200, Stefan Sperling wrote:
> On Tue, Jun 23, 2020 at 10:50:42AM +0200, Sebastien Marie wrote:
> > ah... so the change I made on regress wasn't the right one to expose my problem :)
> > 
> > new diff for regress.
> > -- 
> > Sebastien Marie
> 
> This patch extends your regress test and fixes the problem.
> 
> I am also changing how the status walk discovers .{git,cvs}ignore files.
> Instead of adding patterns in the status_new (diff_new) callback, add a new
> callback which gets invoked once per directory traversed by the status walk.
> 
> ok?
>  
> diff 2c67d7592929f77e883ff5f84785c6acaa06fa49 fecd16836cdf07f06816d889b1c02d8f31df4e54
> blob - 1ad69b7dff719dd615ae921d1fc94b3ec3fabf39
> blob + a1c9a40aad81a8e24de2e0c06e9c5138b03550b1
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -2687,23 +2688,8 @@ status_new(void *arg, struct dirent *de, const char *p
>  		path = de->d_name;
>  	}
>  
> -	if (de->d_type == DT_DIR) {
> -		int subdirfd = openat(dirfd, de->d_name,
> -		    O_RDONLY | O_NOFOLLOW | O_DIRECTORY);
> -		if (subdirfd == -1) {
> -			if (errno != ENOENT && errno != EACCES)
> -				err = got_error_from_errno2("openat", path);
> -		} else {
> -			err = add_ignores(&a->ignores, a->worktree->root_path,
> -			    path, subdirfd, ".cvsignore");
> -			if (err == NULL)
> -				err = add_ignores(&a->ignores,
> -				    a->worktree->root_path, path,
> -				    subdirfd, ".gitignore");
> -			if (close(subdirfd) == -1 && err == NULL)
> -				err = got_error_from_errno2("close", path);
> -		}
> -	} else if (got_path_is_child(path, a->status_path, a->status_path_len)
> +	if (de->d_type != DT_DIR &&
> +	    got_path_is_child(path, a->status_path, a->status_path_len)
>  	    && !match_ignores(&a->ignores, path))
>  		err = (*a->status_cb)(a->status_arg, GOT_STATUS_UNVERSIONED,
>  		    GOT_STATUS_NO_CHANGE, path, NULL, NULL, NULL, -1, NULL);
> @@ -2713,6 +2699,26 @@ status_new(void *arg, struct dirent *de, const char *p
>  }
>  
>  static const struct got_error *
> +status_traverse(void *arg, const char *path, int dirfd)
> +{
> +	const struct got_error *err = NULL;
> +	struct diff_dir_cb_arg *a = arg;
> +
> +	if (a->no_ignores)
> +		return NULL;
> +
> +	err = add_ignores(&a->ignores, a->worktree->root_path,
> +	    path, dirfd, ".cvsignore");
> +	if (err)
> +		return err;
> +
> +	err = add_ignores(&a->ignores, a->worktree->root_path, path,
> +	    dirfd, ".gitignore");
> +

there is a behaviour change in ignore files:
- before, it would use .cvsignore, and if not exists use .gitignore
- now, it will use both .cvsignore and .gitignore

I am not sure if it is intented or not. but I am fine with both :)

> +	return err;
> +}
> +
> +static const struct got_error *
>  report_single_file_status(const char *path, const char *ondisk_path,
>  struct got_fileindex *fileindex, got_worktree_status_cb status_cb,
>  void *status_arg, struct got_repository *repo, int report_unchanged)

Else, the patch reads OK.

Thanks.
-- 
Sebastien Marie