Bug 2152666

Summary: dfu-programmer: C99 issue in configure due to bug in libusb check
Product: [Fedora] Fedora Reporter: Florian Weimer <fweimer>
Component: dfu-programmerAssignee: Florian Weimer <fweimer>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: cameron, igor.raits, weston_schmidt
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: dfu-programmer-0.7.2-12.fc38 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-12-12 17:30:50 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 2137512    
Attachments:
Description Flags
dfu-programmer-c99.patch none

Description Florian Weimer 2022-12-12 16:13:05 UTC
Upstream fixed this by removing old libusb support altogether, but this change is not really backportable, and requires re-running autoconf anyway.

I'm not sure how to deal with this. It is possible to fix it by patching configure directly:

diff --git a/configure b/configure
index 77c9bef30edcbd2e..d4fcdf174a7fcd14 100755
--- a/configure
+++ b/configure
@@ -4596,6 +4596,7 @@ else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 $ac_includes_default
+#include <string.h>
 int
 main ()
 {

Maybe I should do just that, and rely on the fact that rawhide autoconf cannot handle the current configure.ac (without the upstream fixes), so it's unlike this is getting overwritten by accident.

Comment 1 Florian Weimer 2022-12-12 17:29:10 UTC
Created attachment 1932123 [details]
dfu-programmer-c99.patch

Other C99 fix, submitted to upstream.

Comment 2 Florian Weimer 2022-12-12 17:30:50 UTC
I've pushed the configure kludge to rawhide after all. I think the patch for configure serves as a sufficient aid to future maintainer attention.

Comment 3 Cameron Tacklind 2022-12-13 06:17:49 UTC
I'm a new maintainer on dfu-programmer, the one pushing these changes.

I'd be happy to fix any accidentally broken include bugs immediately. I have been setting up a lot of CI on GitHub Actions to ensure at least some level of continuous testing - even though the fast majority of this depends on hardware that is difficult to replicate.

Were you still depending on libusb0 for a reason? Was that a major issue?

If there are any includes that should be added, the *easiest* is to submit a PR on GitHub. I can approve simple changes from my phone, although a version bump is still not automated...

Comment 4 Florian Weimer 2022-12-13 06:30:30 UTC
(In reply to Cameron Tacklind from comment #3)
> I'm a new maintainer on dfu-programmer, the one pushing these changes.

Welcome! Are you contributing to the Fedora package as well?

> I'd be happy to fix any accidentally broken include bugs immediately. I have
> been setting up a lot of CI on GitHub Actions to ensure at least some level
> of continuous testing - even though the fast majority of this depends on
> hardware that is difficult to replicate.
> 
> Were you still depending on libusb0 for a reason? Was that a major issue?

I'm preparing Fedora for future compilers.

  <https://fedoraproject.org/wiki/Changes/PortingToModernC>
  <https://fedoraproject.org/wiki/Toolchain/PortingToModernC>

Tests indicate that the build will go wrong with such compilers. This is exactly the issue that was described upstream in this ticket:

  Misuse of autoconf features in configure.ac causes multiple problems
  <https://github.com/dfu-programmer/dfu-programmer/issues/61>

And the corresponding Savanna ticket.

> If there are any includes that should be added, the *easiest* is to submit a
> PR on GitHub. I can approve simple changes from my phone, although a version
> bump is still not automated...

I see you are already on the PR I submitted, thanks.

From a Fedora perspective, we would have to rebase the package the latest upstream release, but I can't do such a change as part of the whole-distribution work (there are literally hundreds of packages to fix).

Comment 5 Cameron Tacklind 2022-12-13 06:57:59 UTC
I don't use Fedora personally, so not sure that I can contribute directly. I am however happy to help keep dfu-programmer optimal for all. I know that there are some Fedora related files in the source already that I tried to keep updated, but definitely poorly.

I closed 61 because I'd basically rewritten the configure script and assumed I'd fixed that issue. Was I mistaken?

Is Fedora stuck on 0.7.x? What is preventing moving to 1.x?

Comment 6 Florian Weimer 2022-12-13 13:54:22 UTC
(In reply to Florian Weimer from comment #1)
> Created attachment 1932123 [details]
> dfu-programmer-c99.patch
> 
> Other C99 fix, submitted to upstream.

Based on the upstream discussion, this should no longer be needed after rebase, due to the way the code has evolved.