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

Ask libtool to stop hiding some errors #13086

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

rincebrain
Copy link
Contributor

Motivation and Context

Curiously, for #13083, it did not print the actual error encountered for me, on Fedora 35 or Debian 11, it just printed something like this, even with V=1 (though V=1 prints the full command it ran):

Making all in libzpool
make[3]: Entering directory '/home/rich/zfs_heregoes/lib/libzpool'
  CC       dsl_dataset.lo
make[3]: *** [Makefile:1354: dsl_dataset.lo] Error 1
make[3]: Leaving directory '/home/rich/zfs_heregoes/lib/libzpool'

as opposed to:

../../module/zfs/dsl_dataset.c: In function ‘get_receive_resume_token_impl’:
../../module/zfs/dsl_dataset.c:2394:17: error: ‘redact_snaps’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2394 |                 fnvlist_add_uint64_array(token_nv, "redact_snaps",
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2395 |                     redact_snaps, num_redact_snaps);
      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../module/zfs/dsl_dataset.c:2394:17: error: ‘num_redact_snaps’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
cc1: all warnings being treated as errors

After a bit of staring into the abyss, I found it was libtool eating the error output.

In theory, this flag should make it sometimes kick out errors twice. In practice, I'm okay with getting some twice if it means getting some more than 0 times.

(If someone has a better way to accomplish this, I'm all ears, but from reading the "libtool" script, this seems to be the only flag that influences the behavior...)

Description

Shoves -no-suppress into AM_CPPFLAGS for lib/ - AM_LIBTOOLFLAGS puts it too early in the invocation, LTCFLAGS did not do what I might have hoped, and AM_CPPFLAGS globally breaks all of our cmd/ compiles.

How Has This Been Tested?

Well, it correctly bombs printing that error now, and if you fix the error, it successfully built for me.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Feb 11, 2022
@@ -239,3 +242,4 @@ vdev_raidz_math_powerpc_altivec.l$(OBJEXT): CFLAGS += -maltivec
endif

include $(top_srcdir)/config/CppCheck.am
AM_CFLAGS += -no-suppress
Copy link
Contributor

Choose a reason for hiding this comment

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

Stupid question maybe but is this really supposed to be here twice? (I'm fully prepared for Autohell Weirdness™ to mean Yes, but)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely not, I just echoed it into the end of every file and then moved it up to somewhere reasonable, but I had already put it in this file because it was where I hit the problem initially, so I presumably just didn't go edit it at all.

@szubersk
Copy link
Contributor

szubersk commented Feb 15, 2022

I hit exactly the same problem, Debian testing. What you propose here, @rincebrain, is very fragile. I don't see better solutions though.

@rincebrain
Copy link
Contributor Author

I'd prefer a better solution, but everything else I tried did not do the right thing. :/

@@ -27,6 +27,9 @@ AM_CFLAGS += $(ZLIB_CFLAGS)

AM_CFLAGS += -DLIB_ZPOOL_BUILD

# libtool flag
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest adding a comment that this is not really meant for a compiler but libtool and it circumvents autotools deficiencies. It is a gigantic hack but still better than people wasting time on troubleshooting failed compilations.
Should we report this to Mr. Stallman?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

I suspect Mr. Stallman would respond with rude words for daring to not use the GPL on something.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just wrote following to bug-automake@gnu.org


Dear automake developers!

A standard libtool invocation line generated by automake looks like:

LTCOMPILE = $(LIBTOOL) $(AM_V_lt) --tag=CC $(AM_LIBTOOLFLAGS) \
        $(LIBTOOLFLAGS) --mode=compile $(CC) $(DEFS) \
        $(DEFAULT_INCLUDES) $(INCLUDES) $(AM_CPPFLAGS) $(CPPFLAGS) \
        $(AM_CFLAGS) $(CFLAGS)

Sometimes files compiled using the method above make the compiler emit errors. Those errors are suppressed by default which makes troubleshooting impossible. libtool has a command line option, -no-suppress which can be used to make the compiler verbose. Unfortunately, there is no way to inject that option since libtool demands that it comes after --mode=compile. AM_LIBTOOLFLAGS nor LIBTOOLFLAGS cannot be used for that purpose since "it is too early", according to libtool's command line parser. It is somewhat possible to use AM_CFLAGS for that purpose but then it breaks modes other than --mode=compile.

Is there any way to make libtool more verbose?
Thanks in advance!

Software used:

autoconf (GNU Autoconf) 2.71
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+/Autoconf: GNU GPL version 3 or later
<https://gnu.org/licenses/gpl.html>, <https://gnu.org/licenses/exceptions.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David J. MacKenzie and Akim Demaille.
automake (GNU automake) 1.16.5
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv2+: GNU GPL version 2 or later <https://gnu.org/licenses/gpl-2.0.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Tom Tromey <[tromey@redhat.com](mailto:tromey@redhat.com)>
       and Alexandre Duret-Lutz <[adl@gnu.org](mailto:adl@gnu.org)>.

Copy link
Contributor

@szubersk szubersk left a comment

Choose a reason for hiding this comment

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

I hope this workaround won't bite us back hard.

I reported this issue to automake developers https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54020 Let's hope for their quick and accurate response.

EDIT01
Looks like the bug was confirmed: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54020#8

EDIT02
Solution proposal is there: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54020#11
Now we are looking for a hero to implement it: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54020#14

Looks like we're all better off merging @rincebrain hack in right away.

lib/libshare/Makefile.am Show resolved Hide resolved
@nabijaczleweli
Copy link
Contributor

I got hit by this like two days ago, so I highly agree with your EDIT02 (to merge this and possibly revert it after an autohell hero fixes some perl), but edits don't actually generate notifications. This will, though.

@szubersk
Copy link
Contributor

edits don't actually generate notifications. This will, though.

Good point!

@rincebrain
Copy link
Contributor Author

I feel like "this is a terrible hack, but we're better off merging it for now" might be the subtitle of too many of my PRs...

@behlendorf
Copy link
Contributor

Are we absolutely sure this isn't a side effect of setting AM_LIBTOOLFLAGS = --silent in config/Rules.am, here?

I'd really prefer that we not have to make this hack, I'm concerned it might cause us future problems.

@rincebrain
Copy link
Contributor Author

With dec1eef ~1:

make[3]: Entering directory '/tmp/badideabus/lib/libzpool'
  CC       dsl_dataset.lo
make[3]: *** [Makefile:1355: dsl_dataset.lo] Error 1
make[3]: Leaving directory '/tmp/badideabus/lib/libzpool'
make[2]: *** [Makefile:762: all-recursive] Error 1
make[2]: Leaving directory '/tmp/badideabus/lib'
make[1]: *** [Makefile:928: all-recursive] Error 1
make[1]: Leaving directory '/tmp/badideabus'
make: *** [Makefile:789: all] Error 2
$ git diff
diff --git a/config/Rules.am b/config/Rules.am
index 3146da6ee..b08184cdd 100644
--- a/config/Rules.am
+++ b/config/Rules.am
@@ -21,7 +21,7 @@ DEFAULT_INCLUDES += \
        -I$(top_srcdir)/lib/libspl/include/os/freebsd
 endif

-AM_LIBTOOLFLAGS = --silent
+#AM_LIBTOOLFLAGS = --silent

 AM_CFLAGS  = -std=gnu99 -Wall -Wstrict-prototypes -Wmissing-prototypes
 AM_CFLAGS += -fno-strict-aliasing

(And yes, I ran autogen.sh and configure --enable-debug.)

@behlendorf
Copy link
Contributor

Bummer. In that case, let's go ahead and make the change but simply reference the upstream autoconf issue to keep the Makefile's tidy.

# See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54020
AM_CFLAGS += -no-suppress

For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
@rincebrain
Copy link
Contributor Author

There you go.

If you need a rebase, let me know, but I don't think there's any conflicts...

@behlendorf behlendorf merged commit 6a2dda8 into openzfs:master Mar 31, 2022
BrainSlayer pushed a commit to BrainSlayer/zfs that referenced this pull request Apr 11, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 2, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
beren12 pushed a commit to beren12/zfs that referenced this pull request Sep 19, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 21, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
andrewc12 pushed a commit to andrewc12/openzfs that referenced this pull request Sep 23, 2022
For openzfs#13083, curiously, it did not print the actual error, just
that the compile failed with "Error 1".

In theory, this flag should cause it to report errors twice sometimes.
In practice, I'm pretty okay with reporting some twice if it avoids
reporting some never.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Damian Szuberski <szuberskidamian@gmail.com>
Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes openzfs#13086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants