configure: optional build documentation by mikkoi · Pull Request #212 · libcheck/check

mikkoi · 2019-08-19T18:49:54Z

Fix issue #206

Applied an uncommitted patch by Branden Archer.
https://sourceforge.net/p/check/bugs/101/

Signed-off-by: Mikko Johannes Koivunalho mikko.koivunalho@iki.fi

brarcher · 2019-08-21T06:40:04Z

 	if (infnan != NULL) {
+		int i = MAX_CONVERT_LENGTH - 1;
+		while (i >= 0) /* Initialize buffer, do not use memset (compatility) */
+			iconvert[i--] = '\0';


Why not use memset?

Also, why not do this at the beginning at line 1084?

Should the same be done for fconvert, at least for consistency?

Also (not a requirement, just a request), could you send the patch back to the original author as well?

http://www.jhweiss.de/software/snprintf.html

Function memset() is not used anywhere in libcompat. So I figured maybe someone had already come to the conclusion that memset() is not available at every C installation or other compatibility issue. A little backwards logic here, I know. Just being overly careful.

However, memset() is used in libcheck in three places. If you say that it is okay to use memset(), I'm all for it.

I will move it to the beginning of the function, line 1084. A good idea, since the buffer is also used later in the function.

Same for fconvert? I will change that too.

brarcher · 2019-08-21T06:45:40Z

+ * We need the GNU C version to prevent using pragmas where they are
+ * not supported.
+ */
+#define GCC_VERSION (__GNUC__ * 10000 \


Should this be in libcompat instead? Or, is one goal to reduce/remove that header eventually?

brarcher · 2019-08-21T06:47:16Z

 #define CK_ATTRIBUTE_NORETURN
 #endif /* GCC 2.5 */

+/* Actually GCC 4.6.4 */


Oh, you did move it to the header.

I mean to improve the GCC_VERSION_AT_LEAST(major, minor) macro to GCC_VERSION_AT_LEAST(major, minor, patch) because the patch number is useful in some circumstances, such as here.

brarcher · 2019-08-21T06:48:28Z

I see that the Travis-CI build stopped working on the optional docs change. Does there need to be another example build to check that the docs can still be built, and that the docs can be skipped?

mikkoi · 2019-08-21T07:49:03Z

Commit "configure: optional build documentation" does add a new feature, even if it is only in build. @brarcher , can you add it to Travis?

brarcher · 2019-08-23T06:24:25Z

The script which is run on Travis-CI is the travis.sh script. I'd suggest adding the following to the

"${USE_CMAKE}" = 'NO'

path around line 39:

autoreconf -i || exit 1
./configure ${EXTRA_ARGS} --disable-build-docs|| exit 1
make || exit 1

if [ -f doc/version.texi ]; then
echo "Documentation was generated, but was disabled";
exit 1;
fi

The rest of the test can proceed as normal.

Fix issue libcheck#206 Applied an uncommitted patch by Branden Archer. https://sourceforge.net/p/check/bugs/101/ Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

Add more output when running `./configure`. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

Tweak Travis build to support the new `./configure` switch. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

libcheck#206 Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

mikkoi · 2019-09-01T20:11:42Z

Missing the issue #217 .
Should we postpone?

brarcher · 2019-09-03T00:24:39Z

If you would like to postpone merging this until both CMake and Autotools both have a configurable option to build the docs, that is fine. I'll leave it to your discretion. Just let me know when it is ready to merge.

mikkoi · 2019-09-04T13:54:57Z

The documentation itself needs to be revised. Part of the example needs to be rewritten so show how the new CMake import works. I have improved the file FindCheck.cmake to follow new CMake best practices. It is used to find Check installation from CMake build when the build does not support CMake style export/import or the using build wants to use an earlier (0.12.0 or earlier) Autotools built installation.
I think we should merge this branch now and keep the issue #217 open but close issue #206 .

nmeum · 2019-10-25T15:22:52Z

-    AC_MSG_WARN(tex not installed: cannot rebuild HTML documentation.)
+
+if test "xtrue" = x"$enable_build_docs"; then
+    AC_CHECK_PROGS(TEX, tex, false)


Why does this check for tex instead of makeinfo? The issue you referenced also mentions makeinfo, not tex. Your doc makefile also doesn't seem to call tex explicitly instead it uses the implicit GNU make rule which uses makeinfo.

Thanks for pointing this out, and thanks for your fix in #231

mikkoi force-pushed the is-makeinfo-required branch 2 times, most recently from 49e2c49 to 4ad4f87 Compare August 19, 2019 21:46

brarcher reviewed Aug 21, 2019

View reviewed changes

mikkoi mentioned this pull request Aug 21, 2019

Improve the GCC_VERSION_AT_LEAST(major, minor) macro to GCC_VERSION_AT_LEAST(major, minor, patch) #214

Closed

configure: optional build documentation

8ca1f79

Fix issue libcheck#206 Applied an uncommitted patch by Branden Archer. https://sourceforge.net/p/check/bugs/101/ Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

mikkoi force-pushed the is-makeinfo-required branch from a73137d to 486bddc Compare August 29, 2019 15:00

mikkoi added 3 commits August 29, 2019 18:27

configure: optional build documentation, output

42bf6fd

Add more output when running `./configure`. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

configure: optional build documentation, travis

4842075

Tweak Travis build to support the new `./configure` switch. Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

Add issue libcheck#206 to NEWS

c4e5c3c

libcheck#206 Signed-off-by: Mikko Johannes Koivunalho <mikko.koivunalho@iki.fi>

mikkoi force-pushed the is-makeinfo-required branch from 486bddc to c4e5c3c Compare August 29, 2019 16:27

mikkoi mentioned this pull request Aug 30, 2019

Optional Documentation also for CMake Build #217

Open

Merge branch 'master' into is-makeinfo-required

7e1e40a

Merge branch 'master' into is-makeinfo-required

b9ab595

brarcher merged commit 93aacdf into libcheck:master Sep 8, 2019

brarcher mentioned this pull request Sep 9, 2019

Is makeinfo required? #206

Closed

mikkoi deleted the is-makeinfo-required branch September 9, 2019 18:23

nmeum reviewed Oct 25, 2019

View reviewed changes

nmeum mentioned this pull request Oct 25, 2019

configure: check for makeinfo instead of tex for documentation #231

Merged

Conversation

mikkoi commented Aug 19, 2019

Uh oh!

brarcher Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

brarcher Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

mikkoi Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

brarcher Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

brarcher Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

mikkoi Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

brarcher commented Aug 21, 2019

Uh oh!

mikkoi commented Aug 21, 2019

Uh oh!

brarcher commented Aug 23, 2019

Uh oh!

mikkoi commented Sep 1, 2019

Uh oh!

brarcher commented Sep 3, 2019

Uh oh!

mikkoi commented Sep 4, 2019

Uh oh!

nmeum Oct 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brarcher Oct 26, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nmeum Oct 25, 2019 •

edited

Loading