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

failing to build in Fedora 32 #12

Closed
mmckinst opened this issue Jan 30, 2020 · 12 comments
Closed

failing to build in Fedora 32 #12

mmckinst opened this issue Jan 30, 2020 · 12 comments

Comments

@mmckinst
Copy link

mg 20180408 builds for Fedora 31, but not Fedora 32.

The relevant part of the build where it fails in Fedora 32:

/usr/bin/ld: basic.o:/builddir/build/BUILD/mg-20180408/def.h:757: multiple definition of `rptcount'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:757: first defined here
/usr/bin/ld: basic.o:/builddir/build/BUILD/mg-20180408/def.h:756: multiple definition of `tcdell'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:756: first defined here
/usr/bin/ld: basic.o:/builddir/build/BUILD/mg-20180408/def.h:755: multiple definition of `tcinsl'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:755: first defined here
/usr/bin/ld: basic.o:/builddir/build/BUILD/mg-20180408/def.h:754: multiple definition of `tceeol'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:754: first defined here
/usr/bin/ld: basic.o:/builddir/build/BUILD/mg-20180408/def.h:346: multiple definition of `winch_flag'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:346: first defined here
/usr/bin/ld: bell.o:/builddir/build/BUILD/mg-20180408/def.h:757: multiple definition of `rptcount'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:757: first defined here
/usr/bin/ld: bell.o:/builddir/build/BUILD/mg-20180408/def.h:756: multiple definition of `tcdell'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:756: first defined here
/usr/bin/ld: bell.o:/builddir/build/BUILD/mg-20180408/def.h:755: multiple definition of `tcinsl'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:755: first defined here
/usr/bin/ld: bell.o:/builddir/build/BUILD/mg-20180408/def.h:754: multiple definition of `tceeol'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:754: first defined here
/usr/bin/ld: bell.o:/builddir/build/BUILD/mg-20180408/def.h:346: multiple definition of `winch_flag'; autoexec.o:/builddir/build/BUILD/mg-20180408/def.h:346: first defined here

The builds are:

The relevant logs are:

libbsd was upgraded form 0.9.1 in f31 to 0.10.0 in f32 if that is relevant.

I tried upgrading to 20180927 but it also failed to build in f32 with the same error. 20180927 builds fine in f31 though.

The relevant logs for 20180927 are:

@ulm
Copy link
Contributor

ulm commented Jan 30, 2020

Presumably, Gentoo bug 707450 (reported today) is about the same issue. Compilation fails with gcc-10, or with option -fno-common in older gcc versions.

def.h should declare these global variables as extern, and they should be defined in one of the source files.

@hboetes
Copy link
Owner

hboetes commented Jan 31, 2020 via email

@mmckinst
Copy link
Author

Thanks @ulm !

Fedora has a workaround I've applied so its building on f32.

GCC has some more background and Gentoo has a nice wiki that shows how to fix it .

@mmckinst
Copy link
Author

mmckinst commented Feb 3, 2020

Should this be fixed here or upstream in openbsd?

@hboetes
Copy link
Owner

hboetes commented Feb 3, 2020

@ulm I just tried setting them to extern, which doesn't help.

% make 2>&1 |grep rptc
/home/han/src/mg/kbd.c:454: undefined reference to `rptcount'
/usr/bin/ld: /home/han/src/mg/kbd.c:455: undefined reference to `rptcount'
/home/han/src/mg/undo.c:503: undefined reference to `rptcount'

@mmckinst I don't know. I'll use the workaround for now and see what happens.

Global variables are disrecommended. The latest GCC change is trying to herd people to stop using them. I bet this will be heavily debated.

hboetes added a commit that referenced this issue Feb 3, 2020
ulm added a commit to ulm/mg that referenced this issue Feb 3, 2020
@ulm
Copy link
Contributor

ulm commented Feb 3, 2020

Pull request #13 fixes the problem for me.

Should this be fixed here or upstream in openbsd?

I guess this should be reported to OpenBSD as well.

@mmckinst
Copy link
Author

mmckinst commented Feb 3, 2020

I found an answer on stackoverflow that addresses this: https://stackoverflow.com/questions/58609940/multiple-includes-of-same-header-file-in-project-c-vs-c

I am not a C programmer but following the info given in the stackoverflow answer I got this to compile fine with the following POC patch. I don't understand much beyond the basics of C so I don't know the ramifications of what I've changed but the patch is here if its useful. It seems like this should be fixed in OpenBSD's tree?

diff --git a/GNUmakefile b/GNUmakefile
index 4f14b6a..2f02639 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -46,7 +46,7 @@ CC?=		gcc
 CFLAGS?=	-O2 -pipe
 CFLAGS+=	-g -Wall
 # This is a workaround for https://github.com/hboetes/mg/issues/12
-CFLAGS+=	-fcommon
+CFLAGS+=	-fno-common
 CPPFLAGS=	-DREGEX
 CPPFLAGS+=	-D_GNU_SOURCE
 CPPFLAGS+=	$(BSD_CPPFLAGS)
diff --git a/def.h b/def.h
index 9cb6450..8ded866 100644
--- a/def.h
+++ b/def.h
@@ -348,7 +348,7 @@ void		 ttnowindow(void);
 void		 ttcolor(int);
 void		 ttresize(void);
 
-volatile sig_atomic_t winch_flag;
+extern volatile sig_atomic_t winch_flag;
 
 /* ttyio.c */
 void		 ttopen(void);
@@ -767,10 +767,10 @@ extern char		 prompt[];
 /*
  * Globals.
  */
-int		 tceeol;
-int		 tcinsl;
-int		 tcdell;
-int		 rptcount;	/* successive invocation count */
+extern int		 tceeol;
+extern int		 tcinsl;
+extern int		 tcdell;
+extern int		 rptcount;	/* successive invocation count */
 
 /* https://github.com/hboetes/mg/issues/7#issuecomment-475869095 */
 #if defined(__APPLE__) || defined(__NetBSD__)
diff --git a/kbd.c b/kbd.c
index 06d6c9f..e7ea5ba 100644
--- a/kbd.c
+++ b/kbd.c
@@ -1,5 +1,6 @@
 /*	$OpenBSD: kbd.c,v 1.33 2019/07/03 18:11:07 lum Exp $	*/
 
+int rptcount;
 /* This file is in the public domain. */
 
 /*
diff --git a/main.c b/main.c
index 98d5111..c87dd5e 100644
--- a/main.c
+++ b/main.c
@@ -38,6 +38,7 @@ struct buffer	*bheadp;			/* BUFFER list head	*/
 struct mgwin	*curwp;				/* current window	*/
 struct mgwin	*wheadp;			/* MGWIN listhead	*/
 char		 pat[NPAT];			/* pattern		*/
+volatile sig_atomic_t winch_flag;
 
 #ifndef __dead
 #define __dead __attribute__ ((__noreturn__))
diff --git a/tty.c b/tty.c
index 0b64c4b..3aef632 100644
--- a/tty.c
+++ b/tty.c
@@ -37,6 +37,9 @@
 
 #include "def.h"
 
+int tcinsl;
+int tcdell;
+int tceeol;
 static int	 charcost(const char *);
 
 static int	 cci;

@ulm
Copy link
Contributor

ulm commented Feb 3, 2020

I found an answer on stackoverflow that addresses this: https://stackoverflow.com/questions/58609940/multiple-includes-of-same-header-file-in-project-c-vs-c

I am not a C programmer but following the info given in the stackoverflow answer I got this to compile fine with the following POC patch. I don't understand much beyond the basics of C so I don't know the ramifications of what I've changed but the patch is here if its useful. It seems like this should be fixed in OpenBSD's tree?

Apart from winch_flag being defined in main.c instead of tty.c, this is exactly the same solution as in #13.

@hboetes
Copy link
Owner

hboetes commented Feb 3, 2020

Now all we have to do is send the patch to upstream.

@hboetes
Copy link
Owner

hboetes commented Feb 3, 2020

Done! Let's see what happens.

@fobser
Copy link

fobser commented Feb 9, 2020

I just committed it upstream with a bit of whitespace shuffling.
Thanks!
@hboetes could you please send things like this to the tech mailing list even if you don't
attach a diff, I nearly missed it on misc. Thanks.

@hboetes
Copy link
Owner

hboetes commented Feb 9, 2020

@fobser I'm glad you found @ulm's patch useful. Of course code improvement suggestions should be sent to tech@. I'm getting rusty.

@hboetes hboetes closed this as completed Feb 9, 2020
g-branden-robinson pushed a commit to g-branden-robinson/mg that referenced this issue Oct 27, 2023
g-branden-robinson pushed a commit to g-branden-robinson/mg that referenced this issue Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants