Cask was detected as formula with `--only-formulae` #897

muzimuzhi · 2023-04-02T15:01:53Z

I have a tap providing casks only. Today when I opened muzimuzhi/homebrew-texstudio-beta#16 to update cask texstudio-beta, it was detected as formula hence the workflow failed with log

Run brew test-bot --only-formulae --root-url=https://ghcr.io/v2/muzimuzhi/homebrew-texstudio-beta
==> Using Homebrew/homebrew-test-bot 09eca62 (Merge pull request #896 from carlocab/remove-debug-output)
==> Using Homebrew/brew [4](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:5).0.10-119-g931327df1 (Merge pull request #15116 from ZhongRuoyu/bump-cask-pr-sha256)
==> Testing muzimuzhi/homebrew-texstudio-beta 2e3a69f (Merge 4aed83a1[5](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:6)d3fecf22d961f7a092d764e72b56838 into da40a90d1b055e6c123d1b926eefd2290a241861):

==> Running FormulaeDetect#detect_formulae!
==> git -C /home/linuxbrew/.linuxbrew/Homebrew/Library/Taps/muzimuzhi/homebrew-texstudio-beta fetch origin +refs/heads/master
    url               https://github.com/muzimuzhi/homebrew-texstudio-beta/pull/1[6](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:7)/checks
    tap origin/master da40a90 (texstudio: fix style violations)
    HEAD              2e3a69f (Merge 4aed83a15d3fecf22d961f[7](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:8)a092d764e72b56[8](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:9)38 into da40a[9](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:10)0d1b055e6c[12](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:13)3d1b926eefd2290a241861)
    diff_start_sha1   da40a90d1b055e6c123d1b926eefd2290a241861
    diff_end_sha1     2e3a69fbc6b7a778c4[14](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422#step:8:15)4dbf803957f1e8be2a08

    testing_formulae  muzimuzhi/texstudio-beta/texstudio-beta
    added_formulae    (none)
    modified_formulae muzimuzhi/texstudio-beta/texstudio-beta
    deleted_formulae  (none)
Error: No available formula with the name "muzimuzhi/texstudio-beta/texstudio-beta".
Error: Process completed with exit code 1.

In the past, the log of such step looked like

==> Running FormulaeDetect#detect_formulae!
==> git -C /usr/local/Homebrew/Library/Taps/muzimuzhi/homebrew-texstudio-beta fetch origin +refs/heads/master
    url               https://github.com/muzimuzhi/homebrew-texstudio-beta/pull/15/checks
    tap origin/master 1bac692 (texstudio-beta 4.5.1beta6)
    HEAD              3ff9554 (Merge 099a069fcdc5bc86eea44cb751d716716[12](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4031951927/jobs/6931540308#step:8:13)d8704 into 1bac692b95acf1a2ab4a8d8aa56ed42[13](https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4031951927/jobs/6931540308#step:8:14)dcc0d63)
    diff_start_sha1   1bac692b95acf1a2ab4a8d8aa56ed4213dcc0d63
    diff_end_sha1     3ff9554480db77dd866728012c66f9e5b86853a6

    testing_formulae  (none)
    added_formulae    (none)
    modified_formulae (none)
    deleted_formulae  (none)
All steps passed!

Today's failed workflow run https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589072932/jobs/8103716422
A successful workflow run in the past (on Jan 28, 2023) https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4031951927/jobs/6931540308

The text was updated successfully, but these errors were encountered:

carlocab · 2023-04-02T15:42:00Z

Likely caused by e60740f.

Can you try reverting that commit to see if it fixes your problem? You may need to revert bba212d first.

muzimuzhi · 2023-04-02T16:37:54Z

No luck, see https://github.com/muzimuzhi/homebrew-texstudio-beta/actions/runs/4589702164/jobs/8104777253

Run brew test-bot --only-formulae --root-url=https://ghcr.io/v2/muzimuzhi/homebrew-texstudio-beta
==> Using Homebrew/homebrew-test-bot 07b949e (Revert "Formula: "shard" directory.")
[...]
    testing_formulae  muzimuzhi/texstudio-beta/texstudio-beta
    added_formulae    (none)
    modified_formulae muzimuzhi/texstudio-beta/texstudio-beta
    deleted_formulae  (none)
Error: No available formula with the name "muzimuzhi/texstudio-beta/texstudio-beta".
Error: Process completed with exit code 1.

carlocab · 2023-04-02T16:45:14Z

Hmm. Odd. Can you try going back a few months to see if you can find a commit where this was working properly?

muzimuzhi · 2023-04-02T19:19:13Z

After setting some env vars I managed to run brew test-bot --only-formulae locally, which enables me to debug easier (by adding puts). Then I find it's probably caused by Homebrew/brew@f280ce0. It made tap.formula_file?() method too loose (?) if tap is non-official.

In muzimuzhi/homebrew-texstudio-beta@bb7eac7 I reverted Homebrew/brew@f280ce0, then the checks passed.

Currently,

since my tap has no Formula directory, hence in line 120, formula_path is set to /usr/local/Homebrew/Library/Taps/muzimuzhi/homebrew-texstudio-beta, the path to tap;
then in method diff_formulae, Casks/texstudio-beta.rb is caught as a formula candidate;

finally tap.formula_file?(file) in line 194 returns true thus cask Casks/texstudio-beta.rb is wrongly classified as a formula.

homebrew-test-bot/lib/tests/formulae_detect.rb

Lines 119 to 127 in 09eca62

 if tap && diff_start_sha1 != diff_end_sha1 

 formula_path = tap.formula_dir.to_s 

 @added_formulae += 

 diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "A") 

 modified_formulae += 

 diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "M") 

 @deleted_formulae += 

 diff_formulae(diff_start_sha1, diff_end_sha1, formula_path, "D") 

 end

homebrew-test-bot/lib/tests/formulae_detect.rb

Lines 185 to 198 in 09eca62

 def diff_formulae(start_revision, end_revision, path, filter) 

 return unless tap 

 Utils.safe_popen_read( 

 git, "-C", repository, 

 "diff-tree", "-r", "--name-only", "--diff-filter=#{filter}", 

 start_revision, end_revision, "--", path 

 ).lines.map do |line| 

 file = Pathname.new line.chomp 

 next unless tap.formula_file?(file) 

 tap.formula_file_to_name(file) 

 end.compact 

 end

I'll report this to https://github.com/Homebrew/brew.

PS: Adding an empty Formula sub-directory solves the problem too, but git doesn't account an empty directory as a valid change.

muzimuzhi · 2023-04-02T19:29:46Z

I'll report this to https://github.com/Homebrew/brew.

I think I need a better use case that's easier to run locally and involves a third-party tap and tap.formula_file?(file) method. Do you happen to know such one brew usage?

MikeMcQuaid · 2023-04-03T11:39:10Z

IMO this is the type of thing where we're unlikely to fix this ourselves but will review and help you with a PR.

We don't support using brew test-bot with cask taps, really, but will consider fixes to improve behaviour there.

muzimuzhi · 2023-04-03T17:56:33Z

We don't support using brew test-bot with cask taps, really, ... .

@MikeMcQuaid Perhaps this should be stated in doc for brew tap-new because the support for cask taps once existed, which made the new behavior looks like more a regression than undocumented feature.

It seems the tap class needs a more intelligent formula_file?(file) method that could distinguish between class Foo < Formula ... end and cask "Foo" do ... end.

muzimuzhi · 2023-04-03T19:37:17Z

Came across an easier idea: in formula_file?(file), filter out ruby files that reside in Cask directory or one of its sub-directories.

MikeMcQuaid · 2023-04-04T07:28:27Z

Came across an easier idea: in formula_file?(file), filter out ruby files that reside in Cask directory or one of its sub-directories.

This seems like a good idea 👍🏻

MikeMcQuaid · 2023-04-04T07:34:57Z

Actually, thinking about it more: I've found a one-liner test-bot solution based on the fact that casks can only ever be in one directory. PR opened.

muzimuzhi · 2023-04-04T07:57:10Z

@MikeMcQuaid Thank you! #899 does fix my original problem, see this workflow run.

Still I think tap.formula_file?(file) (in Homebrew/brew) could be tightened because IMO it's wrong to have tap.formula_file?(file) and tap.cask_file?(file) both true for some file. (Sorry a bit off-topic here since I'd like to make sure if what I think is reasonable before opening sth in Homebrew/brew.)

diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb
index 3439ec0c9..fb09122cb 100644
--- a/Library/Homebrew/tap.rb
+++ b/Library/Homebrew/tap.rb
@@ -570,7 +570,7 @@ class Tap
     file = file.expand_path(path)
     return false unless ruby_file?(file)

-    file.to_s.start_with?("#{formula_dir}/")
+    file.to_s.start_with?("#{formula_dir}/") && !file.to_s.start_with?("#{cask_dir}/")
   end

   # return true if given path would present a {Cask} file in this {Tap}.

MikeMcQuaid · 2023-04-04T08:58:00Z

@muzimuzhi Would review a PR for that!

MikeMcQuaid mentioned this issue Apr 4, 2023

formulae_detect: reject casks. #899

Merged

MikeMcQuaid closed this as completed in #899 Apr 4, 2023

MikeMcQuaid closed this as completed in 079d637 Apr 4, 2023

This was referenced Apr 4, 2023

Tighten tap.formula_file?(file) and tap.formula_files to not detect cask file as formula Homebrew/brew#15150

Merged

Revert "Merge pull request #899 from Homebrew/formula_detect_cask" #901

Merged

github-actions bot added the outdated label May 5, 2023

github-actions bot locked as resolved and limited conversation to collaborators May 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cask was detected as formula with `--only-formulae` #897

Cask was detected as formula with `--only-formulae` #897

muzimuzhi commented Apr 2, 2023 •

edited

carlocab commented Apr 2, 2023

muzimuzhi commented Apr 2, 2023

carlocab commented Apr 2, 2023

muzimuzhi commented Apr 2, 2023 •

edited

muzimuzhi commented Apr 2, 2023 •

edited

MikeMcQuaid commented Apr 3, 2023

muzimuzhi commented Apr 3, 2023

muzimuzhi commented Apr 3, 2023

MikeMcQuaid commented Apr 4, 2023

MikeMcQuaid commented Apr 4, 2023

muzimuzhi commented Apr 4, 2023 •

edited

MikeMcQuaid commented Apr 4, 2023

Cask was detected as formula with --only-formulae #897

Cask was detected as formula with --only-formulae #897

Comments

muzimuzhi commented Apr 2, 2023 • edited

carlocab commented Apr 2, 2023

muzimuzhi commented Apr 2, 2023

carlocab commented Apr 2, 2023

muzimuzhi commented Apr 2, 2023 • edited

muzimuzhi commented Apr 2, 2023 • edited

MikeMcQuaid commented Apr 3, 2023

muzimuzhi commented Apr 3, 2023

muzimuzhi commented Apr 3, 2023

MikeMcQuaid commented Apr 4, 2023

MikeMcQuaid commented Apr 4, 2023

muzimuzhi commented Apr 4, 2023 • edited

MikeMcQuaid commented Apr 4, 2023

Cask was detected as formula with `--only-formulae` #897

Cask was detected as formula with `--only-formulae` #897

muzimuzhi commented Apr 2, 2023 •

edited

muzimuzhi commented Apr 2, 2023 •

edited

muzimuzhi commented Apr 2, 2023 •

edited

muzimuzhi commented Apr 4, 2023 •

edited