Skip to content

Conversation

hickford
Copy link

No description provided.

Copy link

gitgitgadget bot commented Sep 10, 2025

There are issues in commit dbe46e5:
libsecret/Makefile: add install target
Commit checks stopped - the message is too short
Commit not signed off

@hickford
Copy link
Author

/submit

Copy link

gitgitgadget bot commented Sep 11, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1970/hickford/libsecret-makefile-v1

To fetch this version to local tag pr-1970/hickford/libsecret-makefile-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1970/hickford/libsecret-makefile-v1

Copy link

gitgitgadget bot commented Sep 11, 2025

On the Git mailing list, Junio C Hamano wrote (reply to this):

"M Hickford via GitGitGadget" <[email protected]> writes:

> From: M Hickford <[email protected]>
>
> Signed-off-by: M Hickford <[email protected]>
> ---
>     libsecret/Makefile: add install target
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1970%2Fhickford%2Flibsecret-makefile-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1970/hickford/libsecret-makefile-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1970
>
>  contrib/credential/libsecret/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/contrib/credential/libsecret/Makefile b/contrib/credential/libsecret/Makefile
> index 97ce9c92fb..6fe70065ab 100644
> --- a/contrib/credential/libsecret/Makefile
> +++ b/contrib/credential/libsecret/Makefile
> @@ -26,3 +26,7 @@ $(MAIN): $(OBJS)
>  
>  clean:
>  	@$(RM) $(MAIN) $(OBJS)
> +
> +install: $(MAIN)
> +	$(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir)
> +	$(INSTALL) -m 755 $(MAIN) $(DESTDIR)$(gitexecdir)
>
> base-commit: 4975ec3473b4bc61bc8a3df1ef29d0b7e7959e87

Hmph, the existing make macros used in the Makefile like $(RM),
$(CC), etc. are all defined in the same Makefile, and the only
things this Makefile includes are config.mak and its autogen variant
if they exist, neither of which are the source of INSTALL or
gitexecdir that are used in the main Makefile by being defined
there.

It seems that existing Makefiles in contrib/ like the one in subtree
and contacts, all define their own.  Perhaps you can mimick them by
adding things like

    prefix ?= /usr/local
    gitexecdir ?= $(prefix)/libexec/git-core

    # this should be set to a 'standard' bsd-type install program
    INSTALL  ?= install

that they commonly add for now to make it work?  Without anything
like that, I cannot quite see how it would work with your patch
alone.

And then later we of course should clean things up by splitting the
definitions done in the main Makefile into a common file that can be
included (e.g. path+tool+definitions.mak file), include it from the
main Makefile, and then have contrib/*/Makefile also include it so
that the duplicated definitions like we see in Makefiles in subtree
and contacts (there may be others; they just were the first hits in
my "git ls-files | grep /Makefile").

Copy link

gitgitgadget bot commented Sep 12, 2025

There are issues in commit f77649d:
libsecret/Makefile: add install target
Commit not signed off

Packaging git-credential-libsecret is slightly more complex than
git-subtree.

Add install target similar to that in contrib/subtree/Makefile.

Signed-off-by: M Hickford <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant