vcsh 2.0.2 #84672

RichiH · 2021-09-04T18:59:32Z

Have you followed the guidelines for contributing?
Have you ensured that your commits follow the commit style guide?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

I don't use OS X nor homebrew, I am upstream. I modeled this PR on what others did, but did it by hand. Should be good to go.

alerque · 2021-09-04T19:23:01Z

This is going to need a little help, just bumping the versions isn't enough because the previous iteration of the formula did not use autotools. We need to find something that does and figure out the right macros to trigger ./configure and make install with Homebrew's presets.

alerque · 2021-09-04T19:29:27Z

This is also the wrong source, you should be pulling the sdist attached to the release, not the git Archive. Less tooling is needed to build the sdist.

alerque · 2021-09-04T19:44:32Z

I don't have system with Homebrew to test this one, but here is a patch that will at least get this closer:

From 662c118a4d630440bcb24490cc20f485933ff989 Mon Sep 17 00:00:00 2001
From: Caleb Maclennan <caleb@alerque.com>
Date: Sat, 4 Sep 2021 22:42:29 +0300
Subject: [PATCH] vcsh: use distribution source, update autotools install

Signed-off-by: Caleb Maclennan <caleb@alerque.com>
---
 Formula/vcsh.rb | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/Formula/vcsh.rb b/Formula/vcsh.rb
index f9d219ec112..9d52361d188 100644
--- a/Formula/vcsh.rb
+++ b/Formula/vcsh.rb
@@ -1,19 +1,28 @@
 class Vcsh < Formula
   desc "Config manager based on git"
   homepage "https://github.com/RichiH/vcsh"
-  url "https://github.com/RichiH/vcsh/archive/refs/tags/v2.0.1.tar.gz"
-  version "2.0.1"
-  sha256 "470acd9a0b7d60439fec151ce88a9de92845c1f0a9ecc6868160fa82569cd3e1"
+  url "https://github.com/RichiH/vcsh/releases/download/v2.0.1/vcsh-2.0.1.tar.xz"
+  sha256 "2769e01c8f2fcdaa32cc9f48e86124f3d7c2e93465c75eff33634fa22a86da68"
   license "GPL-2.0"
 
   bottle do
-    sha256 cellar: :any_skip_relocation, all: "d6adf58cb690fa63602d8056d347bf55dde6ebd090c6f82d1bbbbc78b137d93b"
+    sha256 cellar: :any,                 arm64_big_sur: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    sha256 cellar: :any,                 big_sur:       "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    sha256 cellar: :any,                 catalina:      "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    sha256 cellar: :any,                 mojave:        "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+    sha256 cellar: :any_skip_relocation, x86_64_linux:  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
   end
 
+  uses_from_macos "git"
+
   def install
-    bin.install "vcsh"
-    man1.install "vcsh.1"
-    zsh_completion.install "_vcsh"
+
+    args = %W[
+      --prefix=#{prefix}
+    ]
+
+    system "./configure", *args
+    system "make", "install"
   end
 
   test do
-- 
2.33.0

There may be more to do, but hopefully this gets things moving in the right direction.

You can add me to your Homebrew-core fork if you want me to apply and experiment with this branch.

carlocab · 2021-09-04T20:15:22Z

Unfortunately make install fails with

cp -bf completions/vcsh.bash completions/vcsh
cp -bf completions/vcsh.zsh completions/_vcsh
cp: illegal option -- b
usage: cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file target_file
       cp [-R [-H | -L | -P]] [-fi | -n] [-apvXc] source_file ... target_directory

We could perhaps use coreutils during the build to provide cp, but it would be nice if we could make the Makefile portable.

alerque · 2021-09-04T20:30:56Z

I can make a patch release upstream so the Makefile is more portable. Portability and ease of distro packaging across platforms was a major point of this release cycle. ~~In this case the -b is completely unnecessary. Is the -f portable?~~ The -b flag was to avoid interactive prompts when rebuilding the same source a second time while still assuring the file got updated if needed. I swapped out cp -bf for install which should do the same thing by default across platforms.

alerque · 2021-09-04T20:59:53Z

v2.0.2 should have a quick fix for that portability issue.

Signed-off-by: Richard Hartmann <richih@richih.org> Co-authored-by: Caleb Maclennan <caleb@alerque.com> Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>

carlocab

Thanks @RichiH, @alerque.

BrewTestBot · 2021-09-05T06:05:43Z

🤖 A scheduled task has triggered a merge.

alerque · 2021-09-05T08:24:59Z

Formula/vcsh.rb

+    system "make", "install"
+
+    # Remove references to git shim
+    inreplace bin/"vcsh", %r{#{HOMEBREW_SHIMS_PATH}/[^/]+/super/git}o, "git"


Thanks a lot for the help with this @carlocab. I do have a question about this line though. Are we doing something wrong upstream detecting the path to git at configure time? Or is this to correct for an idiosyncrasy in Homebrew where the compile time path to Git is different than what should be used at run time?

Either way I think there is a better way to do this. If the configure time detection (that uses autotools' built in program detection) is not turning up the correct answer, you can inform it manually with ./configure GIT=/path/to/git or even ./configure GIT=git if using relative paths at runtime is appropriate for the platform. This goes for other tools that are detected as well. That's what bothers me here, if Git is not being detected correctly are other tools?

Also I'm curious why there is no depends_on "git" or uses_from_macos "git" or similar. Shouldn't there be some sort of dependency declared for this to work reliably?

On is this to correct for an idiosyncrasy in Homebrew where the compile time path is different than what should be the run time path?

Yes, this, at least in the case of Git, and possibly any absolute path on Linux. Is there any danger in querying PATH at runtime instead of hardcoding absolute paths into vcsh? It seems to me that the former is more flexible, but perhaps there's a reason you're doing this I don't see.

That's what bothers me here, if Git is not being detected correctly are other tools?

The audit should catch stuff that's definitely wrong, though I'm a bit wary of hardcoding /bin/sed and /bin/grep as the paths to search for these on Linux at runtime.

Also I'm curious why there is no depends_on "git" or uses_from_macos "git" or similar. Shouldn't there be some sort of dependency declared for this to work reliably?

Homebrew itself won't work without Git, so any system with Homebrew installed will also have a system Git installation. Doing depends_on "git" or uses_from_macos "git" means we want to use the git formula (and the latter only on Linux); usually because we need a new enough version of Git for things to work. Is that the case here?

BrewTestBot added the deprecated license Formula uses a deprecated SPDX license which should be updated label Sep 4, 2021

RichiH mentioned this pull request Sep 4, 2021

Help distro packagers over the "hump" RichiH/vcsh#314

Open

7 tasks

carlocab changed the title ~~Update vcsh to 2.0.1~~ vcsh 2.0.1 Sep 4, 2021

carlocab force-pushed the patch-1 branch from da7fb0f to a80cc86 Compare September 4, 2021 20:06

carlocab mentioned this pull request Sep 4, 2021

vcsh: update license #84684

Merged

6 tasks

carlocab added the build failure CI fails while building the software label Sep 4, 2021

alerque mentioned this pull request Sep 4, 2021

Patch makefile for macOS portability RichiH/vcsh#316

Merged

carlocab force-pushed the patch-1 branch 3 times, most recently from 775f7be to 761332d Compare September 5, 2021 04:17

vcsh 2.0.2

8e82c44

Signed-off-by: Richard Hartmann <richih@richih.org> Co-authored-by: Caleb Maclennan <caleb@alerque.com> Co-authored-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>

carlocab force-pushed the patch-1 branch from 761332d to 8e82c44 Compare September 5, 2021 04:30

carlocab changed the title ~~vcsh 2.0.1~~ vcsh 2.0.2 Sep 5, 2021

carlocab approved these changes Sep 5, 2021

View reviewed changes

BrewTestBot closed this in 67810b6 Sep 5, 2021

alerque reviewed Sep 5, 2021

View reviewed changes

github-actions bot added the outdated PR was locked due to age label Oct 6, 2021

github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vcsh 2.0.2 #84672

vcsh 2.0.2 #84672

RichiH commented Sep 4, 2021

alerque commented Sep 4, 2021

alerque commented Sep 4, 2021

alerque commented Sep 4, 2021

carlocab commented Sep 4, 2021 •

edited

Loading

alerque commented Sep 4, 2021 •

edited

Loading

alerque commented Sep 4, 2021

carlocab left a comment

BrewTestBot commented Sep 5, 2021

alerque Sep 5, 2021 •

edited

Loading

carlocab Sep 5, 2021

vcsh 2.0.2 #84672

vcsh 2.0.2 #84672

Conversation

RichiH commented Sep 4, 2021

alerque commented Sep 4, 2021

alerque commented Sep 4, 2021

alerque commented Sep 4, 2021

carlocab commented Sep 4, 2021 • edited Loading

alerque commented Sep 4, 2021 • edited Loading

alerque commented Sep 4, 2021

carlocab left a comment

Choose a reason for hiding this comment

BrewTestBot commented Sep 5, 2021

alerque Sep 5, 2021 • edited Loading

Choose a reason for hiding this comment

carlocab Sep 5, 2021

Choose a reason for hiding this comment

carlocab commented Sep 4, 2021 •

edited

Loading

alerque commented Sep 4, 2021 •

edited

Loading

alerque Sep 5, 2021 •

edited

Loading