Bug 24816 - XORG_RELEASE_VERSION macro has undesired side-effects
Summary: XORG_RELEASE_VERSION macro has undesired side-effects
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Build/Modular (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: low minor
Assignee: Gaetan Nadon
QA Contact: Xorg Project Team
URL:
Whiteboard: 2011BRB_Reviewed
Keywords: janitor
Depends on:
Blocks:
 
Reported: 2009-10-30 08:41 UTC by Gaetan Nadon
Modified: 2011-10-08 05:31 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Gaetan Nadon 2009-10-30 08:41:25 UTC
One role of this macro is to add a suffix (e.g X11R7.5) to the module tarname when the --with-release-version=STRING option is used. It does so by setting:

PACKAGE="$PACKAGE-$RELEASE_VERSION"

The PACKAGE variable is used as root value for many other variables. For example Automake pkgdatadir  pkglibdir  and pkgincludedir are affected which was not intended. 

In addition, some makefiles use $(PACKAGE) to compose file or directory names. Unexpected results occur when the option is used and the cause is difficult to diagnose. 

Public interface (configure --help):

  --with-release-version=STRING
                          Use release version string in package name


Original Macro Requirement - by Alan CooperSmith
---------------------------------------
Excerpt from e-mail:

"The original intent was to have the katamari's be released with special
 versions of the tarballs, with -X11R7.5 included - you can see this plan
 in the xorg-modular archives at:
    http://lists.x.org/archives/xorg-modular/2005-July/000586.html

 But in reality, that was only done for 7.0 and 7.1 before being discarded as
 unuseful, since packagers and builders didn't want to have to change the
 tarball/directory name pattern depending on whether it was a katamari-included
 version or a between-katamaris version, so all just always used the
 non-katamari-tagged tarballs instead."

Solutions
---------

1) Deprecate the option (not the macro) with an abort if used. This leaves the public interface in place, but with a deprecation note.

2) Remove the option altogether which will also remove the public interface

3) Do #1 and then a year or so later later, do #2

The second role of the macro remains intact. It defines variables for major, minor and patchlevel. 

Questions to reviewers:

Is this macro option no longer used?
If so, which solution is preferred?
Any other solution?
Comment 1 Julien Cristau 2009-10-30 09:37:28 UTC
On Fri, Oct 30, 2009 at 08:41:26 -0700, bugzilla-daemon@freedesktop.org wrote:

> Solutions
> ---------
> 
> 1) Deprecate the option (not the macro) with an abort if used. This leaves the
> public interface in place, but with a deprecation note.
> 
> 2) Remove the option altogether which will also remove the public interface
> 
> 3) Do #1 and then a year or so later later, do #2
> 
I'd go for #2, I think.  The option hasn't been used in years.
Comment 2 Gaetan Nadon 2009-10-30 10:18:52 UTC
(In reply to comment #1)
> On Fri, Oct 30, 2009 at 08:41:26 -0700, bugzilla-daemon@freedesktop.org wrote:
> 
> > Solutions
> > ---------
> > 
> > 1) Deprecate the option (not the macro) with an abort if used. This leaves the
> > public interface in place, but with a deprecation note.
> > 
> > 2) Remove the option altogether which will also remove the public interface
> > 
> > 3) Do #1 and then a year or so later later, do #2
> > 
> I'd go for #2, I think.  The option hasn't been used in years.
> 

If I understand correctly, this option was used by X.Org only when supplying tarballs to O/S builders. The distro builders would never use this option themselves, it was a naming scheme to help them select which tarball to build in the regular fashion.

If this option always remained within the confine of X.Org walls, then it can be removed safely, and even be reintroduced later without breaking anyone's script.

All 251 modules have this option and for many of them, it's the only one and it'll be deprecated. And it's only useful (was) to the X.Org tarball supplier. I think it's a strong case for removing it.

Alan?

Thanks



Comment 3 Alan Coopersmith 2009-10-30 10:22:37 UTC
(In reply to comment #2)
> If this option always remained within the confine of X.Org walls, then it can
> be removed safely, and even be reintroduced later without breaking anyone's
> script.
> 
> All 251 modules have this option and for many of them, it's the only one and
> it'll be deprecated. And it's only useful (was) to the X.Org tarball supplier.
> I think it's a strong case for removing it.
> 
> Alan?

I certainly don't know of anyone still using it or any reason to keep it.
We'd discussed on xorg-devel a few months ago removing XORG_RELEASE_VERSION
entirely for xorg-macros-1.3, but kept it in because of the useful version
variable settings that got added to it after the original release.
Comment 4 Jeremy Huddleston Sequoia 2011-10-07 17:35:59 UTC
Since nobody is still using this, can we close it out, or do you want to keep 
this open for eventual removal?  We should decide now how long we want to keep 
it around (and deprecated).
Comment 5 Gaetan Nadon 2011-10-08 05:31:03 UTC
The offending part has been removed, the variable definitions remain.

# XORG_RELEASE_VERSION
# --------------------
# Defines PACKAGE_VERSION_{MAJOR,MINOR,PATCHLEVEL} for modules to use.
 
AC_DEFUN([XORG_RELEASE_VERSION],[
	AC_DEFINE_UNQUOTED([PACKAGE_VERSION_MAJOR],
		[`echo $PACKAGE_VERSION | cut -d . -f 1`],
		[Major version of this package])
	PVM=`echo $PACKAGE_VERSION | cut -d . -f 2 | cut -d - -f 1`
	if test "x$PVM" = "x"; then
		PVM="0"
	fi
	AC_DEFINE_UNQUOTED([PACKAGE_VERSION_MINOR],
		[$PVM],
		[Minor version of this package])
	PVP=`echo $PACKAGE_VERSION | cut -d . -f 3 | cut -d - -f 1`
	if test "x$PVP" = "x"; then
		PVP="0"
	fi
	AC_DEFINE_UNQUOTED([PACKAGE_VERSION_PATCHLEVEL],
		[$PVP],
		[Patch version of this package])
])


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.