Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails if tidy-html5 is already installed #2

Closed
eserte opened this issue Oct 26, 2018 · 21 comments · Fixed by #3
Closed

Fails if tidy-html5 is already installed #2

eserte opened this issue Oct 26, 2018 · 21 comments · Fixed by #3
Assignees
Labels
help wanted Extra attention is needed

Comments

@eserte
Copy link

eserte commented Oct 26, 2018

The test suite fails on my FreeBSD smokers, see http://matrix.cpantesters.org/?dist=Alien-TidyHTML5%20v0.1.1;os=freebsd;reports=1

But it seems to pass if I deinstall the tidy-html5 package.

@robrwo
Copy link
Owner

robrwo commented Oct 26, 2018

Could this be related to #1?

@eserte
Copy link
Author

eserte commented Oct 26, 2018

Don't know. If it helps, here's the list of files installed by the package:

tidy-html5-5.6.0:
        /usr/local/bin/tidy5
        /usr/local/include/tidy.h
        /usr/local/include/tidybuffio.h
        /usr/local/include/tidyenum.h
        /usr/local/include/tidyplatform.h
        /usr/local/lib/libtidy.so.5
        /usr/local/lib/libtidy.so.5.6.0
        /usr/local/lib/libtidy5.so
        /usr/local/lib/libtidys.a
        /usr/local/libdata/pkgconfig/tidy.pc
        /usr/local/man/man1/tidy5.1.gz
        /usr/local/share/licenses/tidy-html5-5.6.0/LICENSE
        /usr/local/share/licenses/tidy-html5-5.6.0/MIT
        /usr/local/share/licenses/tidy-html5-5.6.0/catalog.mk

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

The problem seems to be the run_ok method in Test::Alien https://metacpan.org/pod/Test::Alien#run_ok

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

@eserte Is the package named "tidy5"? (See #1)

@eserte
Copy link
Author

eserte commented Oct 28, 2018

The package name is "tidy-html5":

$ pkg info tidy-html5-5.6.0
tidy-html5-5.6.0
Name           : tidy-html5
Version        : 5.6.0
...

@eserte
Copy link
Author

eserte commented Oct 28, 2018

Well, that's the freebsd package name. The pkgconf name is tidy (see above, /usr/local/libdata/pkgconfig/tidy.pc is installed)

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

Hm. The alienfile has it looking for a package named "tidy". Perhaps that's the issue? Can you manually run tests by changing the package name in alienfile and letting me know if it works?

@eserte
Copy link
Author

eserte commented Oct 28, 2018

Well, if the package name is changed (e.g. to tidy5), then the already installed package is not found anymore via pkgconf and things indeed pass. But this is probably not the right solution.

Also it passes if I apply this diff:

diff --git a/t/01-tidy.t b/t/01-tidy.t
index 5b33645..38cc4fa 100644
--- a/t/01-tidy.t
+++ b/t/01-tidy.t
@@ -3,7 +3,7 @@ use Test::Alien;
 use Alien::TidyHTML5;
 
 alien_ok 'Alien::TidyHTML5';
-run_ok( [qw/ tidy -version /] )
+run_ok( [qw/ tidy5 -version /] )
     ->success
     ->out_like(qr/^HTML Tidy /);
 

But this is also not the right fix, as it would start to fail on other OS.

Maybe Alien::TidyHTML5 should know how the executable is named and provide a function to return either /usr/bin/tidy or /usr/local/bin/tidy5 or so?

robrwo added a commit that referenced this issue Oct 28, 2018
On some systems (FreeBSD), the executable may be named "tidy5" instead
of "tidy". So we add this method to look for the first executable.

This also cleans the namespace, and enables clean namespace tests.
@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

I've just pushed some code to the repo that may fix this issue. Can you test it to see if that fixes it?

@robrwo robrwo self-assigned this Oct 28, 2018
@eserte
Copy link
Author

eserte commented Oct 28, 2018

/usr/perl5.24.2p/bin/dzil test on a freebsd 10 system is successful (checked out commit is 907d2ba ). Though it seems that the installed package is ignored now and tidy is downloaded and built from scratch.

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

@eserte Can you try it with commit 8b89d44 reverted, and see if that passes but does not download a new version?

@eserte
Copy link
Author

eserte commented Oct 28, 2018

With this commit reverted (and the conflict in the Changes file resolved) it does not pass (and it seems that there's no download):

t/01-tidy.t .................. 1/?
# Failed test 'exe_file'
# at t/01-tidy.t line 7.
Use of uninitialized value $command[0] in join or string at /usr/perl5.24.2p/lib/site_perl/5.24.2/Test/Alien.pm line 81.

# Failed test 'run  -version'
# at t/01-tidy.t line 9.
#   command not found

# Failed test 'command succeeded'
# at t/01-tidy.t line 9.
#   command not found

# Failed test 'output matches (?^:^HTML Tidy )'
# at /usr/perl5.24.2p/lib/site_perl/5.24.2/Test/Alien/Run.pm line 92.
#   out:
#   does not match:
#     (?^:^HTML Tidy )
# Seeded srand with seed '20181028' from local date.
t/01-tidy.t .................. Dubious, test returned 4 (wstat 1024, 0x400)
Failed 4/5 subtests

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

Hm, that is very strange.

@eserte
Copy link
Author

eserte commented Oct 28, 2018

'run -version' --- this looks like the executable name is missing here.

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

Ok, that means that it's not finding anything. Can you modify the test to output the value of Alien::TidyHTML5->bin_dir?

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

(And then of course let me know if there's anything in that directory)

@plicease
Copy link
Contributor

I think this is not quite right, the PkgConfig plugin should update these values so the gather section shouldn't be necessary.

https://github.com/robrwo/Alien-TidyHTML5/blob/master/alienfile#L37-L41

I can take a closer look at this later if it hasn't been resolved.

@eserte
Copy link
Author

eserte commented Oct 28, 2018

@robrwo : the output of bin_dir is undef:

$ /usr/perl5.24.2p/bin/perl -MAlien::TidyHTML5 -e 'warn Alien::TidyHTML5->bin_dir'
Warning: something's wrong at -e line 1.

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

I'm also wondering if it's the -x test that is failing for some reason.

@robrwo
Copy link
Owner

robrwo commented Oct 28, 2018

@plicease That would be appreciated.

@plicease
Copy link
Contributor

I think #3 should fix this. Please see the notes in that PR for details.

@robrwo robrwo closed this as completed in #3 Oct 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants