Re: [RFC PATCH v2 2/7] object: move function to object.c

[Junio C Hamano <gitster@pobox.com>]

Calvin Wan <calvinwan@google.com> writes:

> While remove_or_warn() is a simple ternary operator to call two other
> wrapper functions, it creates an unnecessary dependency to object.h in
> wrapper.c. Therefore move the function to object.[ch] where the concept
> of GITLINKs is first defined.

An untold assumption here is that we would want to make wrapper.[ch]
independent of Git's internals?

If so, where the thing is moved to (i.e. object.c) is much less
interesting than the fact that the goal of this function is to make
wrapper.[ch] less dependent on Git, so the title should reflect
that, no?

> +/*
> + * Calls the correct function out of {unlink,rmdir}_or_warn based on
> + * the supplied file mode.
> + */
> +int remove_or_warn(unsigned int mode, const char *path);

OK.  That "file mode" thing is not a regular "struct stat .st_mode",
but knows Git's internals, hence it makes sense to have it on our
side, not on the wrapper.[ch] side.  That makes sense.

>  #endif /* OBJECT_H */
> diff --git a/wrapper.c b/wrapper.c
> index 22be9812a7..118d3033de 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -5,7 +5,6 @@
>  #include "abspath.h"
>  #include "config.h"
>  #include "gettext.h"
> -#include "object.h"
>  #include "repository.h"
>  #include "strbuf.h"
>  #include "trace2.h"
> @@ -647,11 +646,6 @@ int rmdir_or_warn(const char *file)
>  	return warn_if_unremovable("rmdir", file, rmdir(file));
>  }
>  
> -int remove_or_warn(unsigned int mode, const char *file)
> -{
> -	return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
> -}
> -
>  static int access_error_is_ok(int err, unsigned flag)
>  {
>  	return (is_missing_file_error(err) ||
> diff --git a/wrapper.h b/wrapper.h
> index c85b1328d1..272795f863 100644
> --- a/wrapper.h
> +++ b/wrapper.h
> @@ -111,11 +111,6 @@ int unlink_or_msg(const char *file, struct strbuf *err);
>   * not exist.
>   */
>  int rmdir_or_warn(const char *path);
> -/*
> - * Calls the correct function out of {unlink,rmdir}_or_warn based on
> - * the supplied file mode.
> - */
> -int remove_or_warn(unsigned int mode, const char *path);
>  
>  /*
>   * Call access(2), but warn for any error except "missing file"