Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

tidy-html5 4.9.25 #39010

Closed
wants to merge 2 commits into from
Closed

tidy-html5 4.9.25 #39010

wants to merge 2 commits into from

Conversation

rstacruz
Copy link
Contributor

end

test do
system bin/"tidy5", "--version"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to write a test that tests the actual functionality?

@mistydemeo
Copy link
Member

brew audit suggests a few revisions:

== /usr/local/Library/Formula/tidy-html5.rb ==
C:  1:  9: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
C:  4: 12: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
C:  5:  7: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
C:  8: 14: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

1 file inspected, 4 offenses detected
Error: 5 problems in 1 formulae

==> audit problems
tidy-html5:
 * Stable: version 4.9.25 is redundant with version scanned from URL
 * cmake dependency should be
  depends_on "cmake" => :build
Or if it is indeed a runtime denpendency
  depends_on "cmake" => :run
 * `require 'formula'` is now unnecessary
 * Use `system "make", "install"` instead of `system "make install"` 
 * A top-level "man" directory was found
Homebrew requires that man pages live under share.
This can often be fixed by passing "--mandir=#{man}" to configure.

@jacknagel
Copy link
Contributor

https://github.com/Homebrew/homebrew-dupes/blob/master/tidy.rb should be addressed if this is merged.

@MikeMcQuaid
Copy link
Member

Could you also add a PR to remove https://github.com/Homebrew/homebrew-dupes/blob/master/tidy.rb? Thanks!

@MikeMcQuaid
Copy link
Member

@rstacruz
Copy link
Contributor Author

Updated it... unfortunately i have no idea how to deal with the man page!

Also, squashed and rebased against the latest head.

@MikeMcQuaid
Copy link
Member

Still failing audit rules: http://bot.brew.sh/job/Homebrew%20Pull%20Requests/24855/version=yosemite/testReport/junit/brew-test-bot/yosemite/audit_tidy_html5___strict/

You can see these locally with brew audit --strict tidy-html5

@rstacruz
Copy link
Contributor Author

absolutely—i've no clue how to fix them though, so I'd appreciate some help.

depends_on "cmake" => :build

def install
ENV.deparallelize
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please submit this as an issue to the upstream developers of this project and add a link to the upstream patch submission and explanation of why the patch is needed in a comment in the formula file. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added this defensively not really knowing if it's necessary... it seems it'll do fine with parallel builds.

@rstacruz rstacruz force-pushed the patch-1 branch 2 times, most recently from eea48e5 to ce92d8e Compare April 26, 2015 04:12
@rstacruz
Copy link
Contributor Author

Updated. This should fix all outstanding issues.

  • now using tidy-html5 4.9.26.
  • cmake now uses *std_make_args.
  • the man issue has been fixed.
  • a more thorough test block has been added.


def install
cd "build/cmake"
puts std_cmake_args
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove this debug print?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops. done

depends_on "cmake" => :build

def install
cd "build/cmake"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to change directory here? What happens if just run in the root of the repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually it is, according to tidy's docs. Can confirm: removing that line will cause a build failure.

   def install
-    cd "build/cmake"
-    system "cmake", "../..", *std_cmake_args
+    system "cmake", *std_cmake_args
     system "make"
     system "make", "install"
   end
==> Downloading https://github.com/htacg/tidy-html5/archive/4.9.26.tar.gz
Already downloaded: /Library/Caches/Homebrew/tidy-html5-4.9.26.tar.gz
==> cmake -DCMAKE_C_FLAGS_RELEASE= -DCMAKE_CXX_FLAGS_RELEASE= -DCMAKE_INSTALL_PREFIX=/usr/local/Cellar/tidy-html5/4.9.26 -DCMAKE_BUILD_T
==> make
warning: failed to load external entity "../documentation/tidy1.xsl"
cannot parse ../documentation/tidy1.xsl
make[2]: *** [man] Error 4
make[1]: *** [CMakeFiles/man.dir/all] Error 2
make: *** [all] Error 2
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1

READ THIS: https://git.io/brew-troubleshooting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird, thanks.

@MikeMcQuaid
Copy link
Member

Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

jdeebee referenced this pull request in SublimeLinter/SublimeLinter-html-tidy May 19, 2015
ghost pushed a commit to SublimeLinter/SublimeLinter-html-tidy that referenced this pull request May 19, 2015
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants