Skip to content

Conversation

@sunweaver
Copy link
Member

This is the first of finally 5 PRs that bring autoreconf build flow into the different parts of nx-libs.

@sunweaver sunweaver requested a review from Ionic June 30, 2017 12:16
@sunweaver sunweaver added this to the 3.6.1.0 milestone Jun 30, 2017
@sunweaver sunweaver force-pushed the pr/nxproxy-autoreconf branch 2 times, most recently from 3df3541 to 4d09a56 Compare June 30, 2017 12:42
m4/nx-macros.m4 Outdated
]
)

[xorg_testset_save_]PREFIX[FLAGS]="$PREFIX[FLAGS]"
Copy link
Member

Choose a reason for hiding this comment

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

xorg_[...] instead of nx_[...] here and all the way down.

Copy link
Member Author

Choose a reason for hiding this comment

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

xorg_[...] instead of nx_[...] here and all the way down.

Yeah, got that.

m4/nx-macros.m4 Outdated

PREFIX[FLAGS]="$PREFIX[FLAGS] ]flag["

# Some hackery here since AC_CACHE_VAL can't handle a non-literal varname
Copy link
Member

Choose a reason for hiding this comment

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

Upstream uses dnl instead of # here - the basic difference being that a line preceded by dnl is discarded completely (i.e., won't show up in the generated output), while #-style comments will be included verbatim in the generated output with the shell later on ignoring that line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Upstream uses dnl instead of # here...

Thanks for the explanation. I wasn't aware of that. I have adapted the nx-macros.m4 file now appropriately, I hope. Having some "#" comments and some dnl comments.

m4/nx-macros.m4 Outdated
AC_SUBST([BASE_]PREFIX[FLAGS])
]) # NX_COMPILER_FLAGS

# Check to see if we're running under Cygwin32.
Copy link
Member

Choose a reason for hiding this comment

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

Probably better to keep these comments as dnl, too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably better to keep these comments as dnl, too.

Done

m4_define([nxproxy_version], m4_esyscmd([tr -d '\n' < VERSION]))

# Initialize Autoconf
AC_PREREQ(2.60)
Copy link
Member

Choose a reason for hiding this comment

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

We're raising the autoconf requirement from 2.13 to 2.60 here. Might be totally okay, but might also cause trouble on older distros (like The QVD or @uli42 use.)

Copy link
Member Author

@sunweaver sunweaver Jul 10, 2017

Choose a reason for hiding this comment

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

> SUSE 11.2 (which is the oldest target distro to build on) has autoconf 2.63. So leaving this.

CXXFLAGS="$CXXFLAGS -DVERSION=\\\"${VERSION}\\\""
CPPFLAGS="$CPPFLAGS -DVERSION=\\\"${VERSION}\\\""

dnl Check whether --with-symbols or --without-symbols was
Copy link
Member

Choose a reason for hiding this comment

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

We're losing the --with-symbols flag and functionality here. Debian builds by default make use of that.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're losing the --with-symbols flag and functionality here. Debian builds by default make use of that.

Good catch. And still... If you look at the current build logs, we have "-g" twice in the options string. One stemming from --with-symbols, the other being added by debhelper. So we can drop this one here.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're losing the --with-symbols...

Removed --with-symbols from CONFIGURE variable in debian/rules now.

@@ -0,0 +1,9 @@
NULL =

man_MANS = \
Copy link
Member

Choose a reason for hiding this comment

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

Changing that to dist_man_MANS should better reflect what you are trying to do - and the EXTRA_DIST definition can be also dropped when using dist_man_MANS.

-L$(top_srcdir)/../nxcomp/ -lXcomp \
$(NULL)

AM_CFLAGS = \
Copy link
Member

Choose a reason for hiding this comment

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

AM_CPPFLAGS should be more appropriate.

m4/nx-macros.m4 Outdated
@@ -0,0 +1,314 @@
# nx-macros.m4. Generated from xorg-macros.m4.in xorgversion.m4 by configure.
Copy link
Member

Choose a reason for hiding this comment

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

Do we really have to duplicate X.Org code? Can't we depend upon and use util-macros directly for the few functions that we need? I understand that doing that might be more difficult (for now - at some point we'll have to use util-macros within the X.Org code base anyway) and that copying makes the NX components more independent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we really have to duplicate X.Org code? Can't we depend upon and use util-macros directly for the few functions that we need? I understand that doing that might be more difficult (for now - at some point we'll have to use util-macros within the X.Org code base anyway) and that copying makes the NX components more independent.

Of course, we can use util-marcos directly. Or rather, we could. However, that let's our builds fail on all older platforms where our customers and friends may need nx-libs to build.

@sunweaver sunweaver force-pushed the pr/nxproxy-autoreconf branch from 4d09a56 to c9b1e15 Compare July 10, 2017 10:11
@sunweaver
Copy link
Member Author

@Ionic: next iteration for #473.

@sunweaver
Copy link
Member Author

sunweaver commented Jul 10, 2017

@Ionic: most of the commits will be flattened into one at the end (except the --with-symbols drop from debian/rules).

Update: Flattened into two commits (the nx-macros.m4 file and changes to nxproxy/). So three commits for this PR with the change to debian/rules.

@sunweaver sunweaver force-pushed the pr/nxproxy-autoreconf branch from 35731a0 to 06d86a9 Compare July 12, 2017 11:11
 This also solves the last remnant of overlinking as described in GH issue ArcticaProject#133.

 Fixes ArcticaProject#133.
@sunweaver sunweaver force-pushed the pr/nxproxy-autoreconf branch from 06d86a9 to 397d3ac Compare July 12, 2017 21:27
@Ionic Ionic merged commit 397d3ac into ArcticaProject:3.6.x Jul 12, 2017
Ionic added a commit that referenced this pull request Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants