Bug 9368

Summary: non portable sed usage in xorgversion.m4
Product: xorg Reporter: Matthieu Herrb <matthieu.herrb>
Component: Build/ModularAssignee: Xorg Project Team <xorg-team>
Status: RESOLVED FIXED QA Contact: Xorg Project Team <xorg-team>
Severity: normal    
Priority: high    
Version: 7.2 (2007.02)   
Hardware: x86 (IA32)   
OS: OpenBSD   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 6666    
Attachments:
Description Flags
proposed patch: use 'cut' instead. none

Description Matthieu Herrb 2006-12-16 11:26:34 UTC
BSD and Solaris sed use basic regular expressions. That meanss that [^.]+
construct used in xorgversion.m4 is doubly non portable.
Comment 1 Matthieu Herrb 2006-12-16 11:27:38 UTC
Created attachment 8140 [details] [review]
proposed patch: use 'cut' instead.
Comment 2 Matthieu Herrb 2006-12-16 11:29:17 UTC
Would be nice to fix for 7.2. xf86-video-nv 1.2.1 doesn't build on systems with
non-cgnu sed without this.
Comment 3 Alan Coopersmith 2006-12-18 18:28:15 UTC
Thank you - this fixes the xf86-video-nv build failure on Solaris.

I've committed the fix to git, but the bug isn't really fixed until the packages
which use it are regenerated with the new macros installed, so I'm not marking as
fixed yet.
Comment 4 Andres Salomon 2006-12-18 18:45:33 UTC
(In reply to comment #1)
> Created an attachment (id=8140) [edit]
> proposed patch: use 'cut' instead.
> 

The patch was written to work w/ shorter version strings.  The 'cut' version of
this patch doesn't work correctly:

dilinger@sticky:~/Desktop$ echo 1.2.3 | cut -d . -f 3
3
dilinger@sticky:~/Desktop$ echo 1.2 | cut -d . -f 3

dilinger@sticky:~/Desktop$ echo 1 | cut -d . -f 3
1


That last one should be ' ', not '1'.  I think what we want here is 'cut -s'. 
How portable is that?
Comment 5 Alan Coopersmith 2006-12-18 19:26:01 UTC
cut -s works on Solaris:

alanc@alf:~ [7:25pm - 203] echo 1.2.3 | cut -s -d . -f 3
3
alanc@alf:~ [7:25pm - 204] echo 1.2 | cut -s -d . -f 3

alanc@alf:~ [7:25pm - 205] echo 1 | cut -s -d . -f 3
alanc@alf:~ [7:25pm - 206] 
Comment 6 Daniel Stone 2006-12-18 22:32:12 UTC
(In reply to comment #3)
> I've committed the fix to git, but the bug isn't really fixed until the packages
> which use it are regenerated with the new macros installed, so I'm not marking as
> fixed yet.

urgh.  that's a hell of a lot of packages.
Comment 7 Matthieu Herrb 2006-12-18 23:32:02 UTC
cut -s is standard. So no objection to use it. 
However if some of the PACKAGE_VERSION_* macros end up empty, things like
drivers that use them to initialize integers in a structure will still fail. We
can leave with that I think. 

Daniel, when I checked the source tarballs that I build for OpenBSD, there are
only a two of them which use the new macros: xf86-video-nv and xf86-video-mga. 
These macros where added pretty recently to xorgversion.m4.
Comment 8 Alan Coopersmith 2006-12-19 07:50:15 UTC
Not all packages - just those that use the definitions - I haven't done a full
scan, but thought that was currently limited to xf86-video-nv & xf86-video-mga.

Since these #defines were just added, not many packages have had time to adopt
them yet.
Comment 9 Daniel Stone 2007-02-27 01:35:13 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 10 Matthieu Herrb 2007-03-07 10:25:16 UTC
packages have been regenerated now. 

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.