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

bug(build): tidyBufAppend(&buf1, d->def, strlen(d->def)); #721

Closed
Kristinita opened this issue Apr 20, 2018 · 5 comments
Closed

bug(build): tidyBufAppend(&buf1, d->def, strlen(d->def)); #721

Kristinita opened this issue Apr 20, 2018 · 5 comments

Comments

@Kristinita
Copy link

Kristinita commented Apr 20, 2018

1. Summary

I get warnings in console, when I build HTML Tidy.

2. Argumentation

I would install HTML Tidy on Ubuntu easily, use sudo apt install tidy, but I get very outdated version.

I need build HTML Tidy for latest version usage. I build HTML Tidy as in this article.

3. Environment

  • Ubuntu 14.04.5 LTS

See more information, if neediest in Travis CI Build system information.

4. Steps to reproduce

See .travis.yml on SashaCompiling branch of my demo repository.

$ git clone https://github.com/htacg/tidy-html5.git
$ cd tidy-html5
$ cd build/cmake
$ cmake ../..
$ cmake --build . --config Release
$ sudo make install

5. Expected behavior

No warnings in console.

6. Actual behavior

See on Travis CI:

[ 98%] Building C object CMakeFiles/tidy.dir/console/tidy.c.o
/home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/console/tidy.c: In functionprintOptionExportValues’:
/home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/console/tidy.c:1565:25: warning: passing argument 2 oftidyBufAppenddiscardsconstqualifier from pointer target type [enabled by default]
                         tidyBufAppend(&buf1, d->def, strlen(d->def));
                         ^
In file included from /home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/console/tidy.c:23:0:
/home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/include/tidybuffio.h:81:28: note: expectedvoid *but argument is of typectmbstrTIDY_EXPORT void TIDY_CALL tidyBufAppend( TidyBuffer* buf, void* vp, uint size );
                            ^
/home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/console/tidy.c:1592:25: warning: passing argument 2 oftidyBufAppenddiscardsconstqualifier from pointer target type [enabled by default]
                         tidyBufAppend(&buf1, d->def, strlen(d->def));
                         ^
In file included from /home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/console/tidy.c:23:0:
/home/travis/build/Kristinita/SashaTidyDebugging/tidy-html5/include/tidybuffio.h:81:28: note: expectedvoid *but argument is of typectmbstrTIDY_EXPORT void TIDY_CALL tidyBufAppend( TidyBuffer* buf, void* vp, uint size );
                            ^

Thanks.

@geoffmcl
Copy link
Contributor

@Kristinita thanks for the issue...

Yes, some distros can sometimes be very slow to catch up to the latest release. That is particularly true for Ubuntu! The last time I checked, even with my 16.04.4 LTS, which you should upgrade to, it is still a tidy 2009 version, libtidy-0.99-0... UGH ...

Even the Jonathon F launchpad backport is still on tidy 5.2.0...

Anything you could do about that like filing an update bug with them would be most appreciated...

And likewise we stopped shipping a binary .deb around the same time...

Thankfully cloning and building tidy from source is usually very simple in linux. So you can benefit from the very latest tidy, and be able to test branches before they are merged...

We have our own BUILD.md. And note the xsltproc tool is required to build and install the tidy.1 man page. $ sudo apt-get install xsltproc should do it...

Note, it appears you did not add -DCMAKE_INSTALL_PREFIX[:PATH]=/usr to the cmake configuration and generation stage, which allows cmake to choose its default, which is usually /usr/local, which is not particularly a problem...

But I consider granddaddy tidy a system app, not just a user local addition... but as I say no real problem...

And I would add -DCMAKE_BUILD_TYPE=Release to that. Yes, it got added to the cmake build line, but in linux you can use just $ make instead... given the standard UNIX makefiles generation, usually the unix default, was used...

And yes, we have allowed two warnings to creep into the compile of tidy.c. Checking them carefully, they are quite benign, and can be ignored... certainly not a bug... But I will try to remember to fix them when I get a chance... always better to have a clean build...

Will leave this open to remind me to do that small fix, probably with a cast... thanks...

@geoffmcl geoffmcl added this to the 5.7 milestone Apr 20, 2018
geoffmcl added a commit that referenced this issue Apr 20, 2018
@geoffmcl
Copy link
Contributor

@Kristinita happened to be working in linux, and experimented with a quick fix for these gcc warnings...

Have pushed the results to the issue-721 branch, and created PR #722...

Appreciate any testing and reporting ... thanks...

@Kristinita
Copy link
Author

If I agree that -DCMAKE_INSTALL_PREFIX[:PATH] value — default path:

    Is a way below recommended for Ubuntu?

$ sudo apt-get install xsltproc
$ git clone https://github.com/htacg/tidy-html5.git
$ cd tidy-html5
$ cd build/cmake
$ cmake ../.. -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIB:BOOL=OFF
$ make
$ sudo make install

Thanks.

Kristinita pushed a commit to Kristinita/KristinitaPelican that referenced this issue Apr 21, 2018
1. Update local Python dependencies — https://docs.pipenv.org/basics/#example-pipenv-upgrade-workflow , 2. Fix sensitive blank lines in CODE_OF_CONDUCT.md — EthicalSource/contributor_covenant#528 , 3. Fix HTML Tidy build warnings — htacg/tidy-html5#721 4. Add exit codes for validation scripts — https://blog.travis-ci.com/after_script_behavior_changes/ .
Kristinita pushed a commit to Kristinita/SashaTidyDebugging that referenced this issue Apr 21, 2018
1. Update local Python dependencies — https://docs.pipenv.org/basics/#example-pipenv-upgrade-workflow , 2. Fix sensitive blank lines in CODE_OF_CONDUCT.md — EthicalSource/contributor_covenant#528 , 3. Fix HTML Tidy build warnings — htacg/tidy-html5#721 , 4. Add exit codes for validation scripts — https://blog.travis-ci.com/after_script_behavior_changes/ .
Kristinita pushed a commit to Kristinita/SashaGitHub that referenced this issue Apr 21, 2018
1. Update local Python dependencies — https://docs.pipenv.org/basics/#example-pipenv-upgrade-workflow

2. Fix sensitive blank lines in CODE_OF_CONDUCT.md — EthicalSource/contributor_covenant#528

3. Fix HTML Tidy build warnings — htacg/tidy-html5#721

4. Add exit codes for validation scripts — https://blog.travis-ci.com/after_script_behavior_changes/
@geoffmcl
Copy link
Contributor

@Kristinita well the recommended way depends on what you are going to do, what you want to do...

If you are just going to use the console app tidy, which by default is linked with the static library libtidys.a, then not building and installing the shared library, libtidy.so, that is -DBUILD_SHARED_LIB:BOOL=OFF, is fine, but this is unusual in unix/linux...

It also means you have no other applications in your system that depend on libtidy.so, like say PHP with tidy, see #673, and perhaps a Perl wrapper that uses libtidy, see #562, etc, but it is true there are not too many of these...

And it also means you do not want or need to write your own applications, like our sample app, or like some other samples in my test-tidy repository, or you absolutely prefer to only link with the static libtidys.a...

There are even some in the unix/linux community that prefer to build the console app tidy linking only with the shared library. See the cmake -DTIDY_CONSOLE_SHARED:BOOL=ON option, and see issue #326...

And further concerning the shared library, and tidy headers, you have to make sure older version are still not installed...

See #707, and several others, especially where version 0.99, circa 2009, is still around. Run something like $ locate tidy, and make sure all older versions have been purged...

And that also means to try to be consistent with the install location, like using -DCMAKE_INSTALL_PREFIX[:PATH]=/usr, but as stated in most cases the cmake default is also fine... and usually does not represent a problem...

In building in Windows, there are other good reason why the shared library, there called a DLL, is less important, but that is because a DLL there takes more effort to get it setup correctly... but not impossible...

So you can see giving a recommendation depends on many other things...

HTH.

Kristinita pushed a commit to Kristinita/KristinitaPelican that referenced this issue Apr 21, 2018
1. Update local Python dependencies — https://docs.pipenv.org/basics/#example-pipenv-upgrade-workflow

2. Fix sensitive blank lines in CODE_OF_CONDUCT.md — EthicalSource/contributor_covenant#528

3. Fix HTML Tidy build warnings — htacg/tidy-html5#721

4. Add exit codes for validation scripts — https://blog.travis-ci.com/after_script_behavior_changes/
geoffmcl added a commit that referenced this issue Apr 24, 2018
Is #721 - cast away some gcc warnings - PR #722
@geoffmcl
Copy link
Contributor

@Kristinita warning fix PR #722 now merged, and got a clean build for version 5.7.14... thanks...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants