Bug#704252: preparing the fix
On Sun, 2013-04-14 at 08:34 +0200, Paul Gevers wrote: > On 14-04-13 01:45, peter green wrote: > > Sorry I could have been clearer in my last mail. I didn't intend to > > blame you for most of the issues with the patch (you just took a > > broken patch and made it differently broken) but I could see how it > > could have come across that way. > > Thanks for the clarification, I appreciate that. I like this end :) thanks. > > Still I firmly belive that the name in the changechangelog trailer > > should be the person who finalised the upload. > > Again, I also doubted. I am watching and contributing on the > d-mentors@l.d.o list and see this happening once in a while. In this > case I wanted to credit Abou for the change, instead, because it failed, > it might look otherwise. Indeed, I am now convinced you are right. Next > time I will credit in the log itself. > > > If noone naks this in the next few days I will go ahead and upload > > it. > > Please go ahead. Lets get rid of this RC bug in Wheezy ASAP, so we can > release. If I can help by filing and tracking the unblock after > successful build (or I can even do the (unchanged :) ) upload for you), > please let me know (here or in private). Sorry for late answer, but It is OK with me too. Cheers, signature.asc Description: This is a digitally signed message part
Bug#704252: preparing the fix
On Fri, 2013-04-12 at 22:48 +0200, Paul Gevers wrote: > > 1) You used dpkg-architecture -qDEB_BUILD_ARCH instead of > > dpkg-architecture -qDEB_BUILD_GNU_CPU (you could simplify your rules > > file as well with this). > > Hmm, it seems like this was not a smart idea for i386. So the point is > that either we have to fix i486 to i386 or amd64 to x86_64... > > Abou, what is your proposal, and what do you prefer? I can upload again > tomorrow. Hi Paul, I'd rather keep what I did as it was the same code used for Lazarus. If there is an issue in the amd64 arch, please report it and I'll try to fix that. There is a translation table in the debian/rules that ensure we get the right arch. This should be probably resolved other way by patching fpcmake and the compiler to conform to debian lib path naming policy. Cheers, signature.asc Description: This is a digitally signed message part
Bug#704252: preparing the fix
Paul Gevers wrote: Please go ahead. Lets get rid of this RC bug in Wheezy ASAP, so we can release. If I can help by filing and tracking the unblock after successful build (or I can even do the (unchanged :) ) upload for you), please let me know (here or in private) I just uploaded, feel free to take it from here. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#704252: preparing the fix
On 14-04-13 01:45, peter green wrote: > Sorry I could have been clearer in my last mail. I didn't intend to > blame you for most of the issues with the patch (you just took a > broken patch and made it differently broken) but I could see how it > could have come across that way. Thanks for the clarification, I appreciate that. > Still I firmly belive that the name in the changechangelog trailer > should be the person who finalised the upload. Again, I also doubted. I am watching and contributing on the d-mentors@l.d.o list and see this happening once in a while. In this case I wanted to credit Abou for the change, instead, because it failed, it might look otherwise. Indeed, I am now convinced you are right. Next time I will credit in the log itself. > If noone naks this in the next few days I will go ahead and upload > it. Please go ahead. Lets get rid of this RC bug in Wheezy ASAP, so we can release. If I can help by filing and tracking the unblock after successful build (or I can even do the (unchanged :) ) upload for you), please let me know (here or in private). Paul signature.asc Description: OpenPGP digital signature
Bug#704252: preparing the fix
Paul Gevers wrote:
Peter,
Please calm down.
Sorry I could have been clearer in my last mail. I didn't intend to
blame you for most of the issues with the patch (you just took a broken
patch and made it differently broken) but I could see how it could have
come across that way. Still I firmly belive that the name in the
changechangelog trailer should be the person who finalised the upload.
If you belive changes are needed then IMO you should either bounce them
back to the person who prepared the upload for comment or make the
upload in your own name.
Anyway i'm proposing the upload given in the attached debdiff (note:
debdiff is against the version in testing, not the version in unstable).
If noone naks this in the next few days I will go ahead and upload it.
diff -Nru fpc-2.6.0/debian/changelog fpc-2.6.0/debian/changelog
--- fpc-2.6.0/debian/changelog 2012-12-22 09:12:58.0 +
+++ fpc-2.6.0/debian/changelog 2013-04-13 08:09:02.0 +
@@ -1,3 +1,18 @@
+fpc (2.6.0-9) unstable; urgency=low
+
+ * Revert previous broken and unnessacerally intrustive fix for debian/control
+issue. Replace it with a more targetted fix inspired by abou's and paul's
+proposed patches.
+
+ -- Peter Michael Green Sat, 13 Apr 2013 07:52:42 +
+
+fpc (2.6.0-8) unstable; urgency=low
+
+ * Removed auto-generation of debian/control during build process as required
+by policy. (Closes: Bug#704252)
+
+ -- Abou Al Montacir Tue, 26 Mar 2013 09:54:00 +0100
+
fpc (2.6.0-7) unstable; urgency=low
* Proofread templates by debian-l10n-english list. (Closes: Bug#686038)
diff -Nru fpc-2.6.0/debian/fixdeb fpc-2.6.0/debian/fixdeb
--- fpc-2.6.0/debian/fixdeb 2012-11-28 07:00:37.0 +
+++ fpc-2.6.0/debian/fixdeb 2013-04-13 08:09:39.0 +
@@ -27,5 +27,8 @@
for i in $1/*.in
do
j=`basename ${i} .in`
- sed ${SUBST_CMD} ${i} > $1/${j/./${PACKAGESUFFIX}.}
+ if ${GEN_CONTROL} || test ${j} != 'control'
+ then
+sed ${SUBST_CMD} ${i} > $1/${j/./${PACKAGESUFFIX}.}
+ fi
done
diff -Nru fpc-2.6.0/debian/rules fpc-2.6.0/debian/rules
--- fpc-2.6.0/debian/rules 2012-11-28 06:31:42.0 +
+++ fpc-2.6.0/debian/rules 2013-04-13 23:02:21.0 +
@@ -169,9 +169,13 @@
debian-files: debian-files-stamp
debian-files-stamp:
@echo "--- Creating/fixing *.install files"
- /bin/bash debian/fixdeb debian $(FPCTARGET) $(PPNEW)
+ GEN_CONTROL=false /bin/bash debian/fixdeb debian $(FPCTARGET) $(PPNEW)
touch debian-files-stamp
+debian/control: debian/control.in
+ GEN_CONTROL=true /bin/bash debian/fixdeb debian $(FPCTARGET) $(PPNEW)
+
+
###
# Arch packages
#
Bug#704252: preparing the fix
Peter,
Please calm down.
1) I made a small change to the what Abou proposed, because his script
did not work on my system (amd64) so most "issues" you mention are
actually proposed by Abou. I think, but he can speak for himself, that
he wanted more or less the same as in Lazarus.
Actually, I really was doubting if I would upload Abou's proposal and
already made a very low impact change instead (see below) but:
2) The release team already approved Abou's changes, so in the end I
decided to upload Abous's proposal, so that I was less "in the way" of
the real maintainers of fpc.
It seems I made a mistake, I will fix it. Do you think belows fix (minus
the version) is better, or do we (I think Abou will like that better) to
fix the new fixdeb with the same substr as in the rules file? Or even
pass the proper string to the script in the rules file (it is already
calculated there).
My proposal is to use my fix, as the rest of the script is proven to
work until now, and Abou can fix his fixdeb script for jessie.
Paul
diff -Nru fpc-2.6.0/debian/changelog fpc-2.6.0/debian/changelog
--- fpc-2.6.0/debian/changelog 2012-12-22 10:12:58.0 +0100
+++ fpc-2.6.0/debian/changelog 2013-04-12 18:49:54.0 +0200
@@ -1,3 +1,11 @@
+fpc (2.6.0-7.1) unstable; urgency=low
+
+ * NMU upload
+ * Removed auto-generation of debian/control during build process as required
+by policy. (Closes: Bug#704252)
+
+ -- Paul Gevers Fri, 12 Apr 2013 18:49:15 +0200
+
fpc (2.6.0-7) unstable; urgency=low
* Proofread templates by debian-l10n-english list. (Closes: Bug#686038)
diff -Nru fpc-2.6.0/debian/fixdeb fpc-2.6.0/debian/fixdeb
--- fpc-2.6.0/debian/fixdeb 2012-11-28 08:00:37.0 +0100
+++ fpc-2.6.0/debian/fixdeb 2013-04-12 18:59:24.0 +0200
@@ -27,5 +27,9 @@
for i in $1/*.in
do
j=`basename ${i} .in`
- sed ${SUBST_CMD} ${i} > $1/${j/./${PACKAGESUFFIX}.}
+ if [ ${j} = 'control' ] ; then
+echo "Skipping $i to prevent Debian bug #704252."
+ else
+sed ${SUBST_CMD} ${i} > $1/${j/./${PACKAGESUFFIX}.}
+ fi
done
signature.asc
Description: OpenPGP digital signature
Bug#704252: preparing the fix
Paul Gevers wrote: I just uploaded with the patch for bug 704252. I didn't feel the other two items appropriate at this moment. It took me some more time than expected, because I had to fix two bugs in your script: So you are saying that rather rather than sponsoring abou's upload you made your own upload and put abou's name on it? If so please don't impersonate maintainers like that. 1) You used dpkg-architecture -qDEB_BUILD_ARCH instead of dpkg-architecture -qDEB_BUILD_GNU_CPU (you could simplify your rules file as well with this). I just got an email from a buildd saying the i386 build had failed and I took a look at this debdiff. The more I read of it the worse it seemed to get. Firstly neither debian or gnu CPU names match the cpu names freepascal uses. So whatever you do some special casing is going to be needed in the determation of the fpc target name . Secondly the free pascal target name was previously passed from debian/rules to debian/fixdeb so it only had to be worked out in one place. Why is that not the case anymore? Thirdly why has the calling syntax for debian/fixdeb been changed? Fourthly shouldn't this be using DEB_HOST_* not DEB_BUILD_* (yeah I know cross-building the fpc package is probablly broken anyway but lets not make it any more broken) Fifthly your debian/control rule seems like it can't possiblly work since it specifies *.in when afaict (under your new calling syntax) it should be debian/*.in Finally is it really nessacery to use obscure make syntax to run the script? In summary if I was a release team member there is no way i'd be unblocking this. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#704252: preparing the fix
> 1) You used dpkg-architecture -qDEB_BUILD_ARCH instead of > dpkg-architecture -qDEB_BUILD_GNU_CPU (you could simplify your rules > file as well with this). Hmm, it seems like this was not a smart idea for i386. So the point is that either we have to fix i486 to i386 or amd64 to x86_64... Abou, what is your proposal, and what do you prefer? I can upload again tomorrow. Paul signature.asc Description: OpenPGP digital signature
Bug#704252: preparing the fix
Hi Abou,
On 11-04-13 00:17, Abou Al Montacir wrote:
> Can you please take 2.6.0 branch head and upload. The other 2 items are
> just a typo in translation file and a fix suggested by release team
> themselves. I think we really conform to the freeze goals. However if
> you don't like these two fixes, please feel free to remove them.
I just uploaded with the patch for bug 704252. I didn't feel the other
two items appropriate at this moment.
It took me some more time than expected, because I had to fix two bugs
in your script:
1) You used dpkg-architecture -qDEB_BUILD_ARCH instead of
dpkg-architecture -qDEB_BUILD_GNU_CPU (you could simplify your rules
file as well with this).
2) Your "if ${1} = '--gen-control'" line failed on one file and also
failed when I was calling fixdeb with the wrong arguments.
Find the debdiff I applied to 2.6.0-7 attached.
Paul
diff -Nru fpc-2.6.0/debian/changelog fpc-2.6.0/debian/changelog
--- fpc-2.6.0/debian/changelog 2012-12-22 10:12:58.0 +0100
+++ fpc-2.6.0/debian/changelog 2013-04-12 20:56:16.0 +0200
@@ -1,3 +1,10 @@
+fpc (2.6.0-8) unstable; urgency=low
+
+ * Removed auto-generation of debian/control during build process as required
+by policy. (Closes: Bug#704252)
+
+ -- Abou Al Montacir Tue, 26 Mar 2013 09:54:00 +0100
+
fpc (2.6.0-7) unstable; urgency=low
* Proofread templates by debian-l10n-english list. (Closes: Bug#686038)
diff -Nru fpc-2.6.0/debian/fixdeb fpc-2.6.0/debian/fixdeb
--- fpc-2.6.0/debian/fixdeb 2012-11-28 08:00:37.0 +0100
+++ fpc-2.6.0/debian/fixdeb 2013-04-12 19:38:14.0 +0200
@@ -3,29 +3,53 @@
# Create debian files from *.in files
#
-#set -x
-if [ $# -lt 3 ]; then
- echo 'Usage : fixdeb[priority]'
- echo ' = path to debian files'
- echo ' = fpc target (i386-linux)'
- echo ' = ppcXXX binary name (ppc386)'
- echo '[priority] = package priority in alternative system'
- exit 1
-fi
+set -e
+
+usage()
+{
+ echo 'Usage : DEB_SUBST_= ... DEB_SUBST_=
fixdeb [-sc|--gen-control] [file1.in] ... [filen.in]'
+ echo ' Changes environment variables with their values. The variables
to be'
+ echo ' changed should be exported prefixed with DEB_SUBST_'
+ echo '--gen-control: do not skip generating control file, by
default it will be ignored'
+ echo ' = space separated list to debian files
templates'
+ echo
+ echo ' List of defined variables'
+ set | grep '^DEB_SUBST_'
+ exit 1
+}
+
+true ${DEB_SUBST_PACKAGEVERSION:=$(dpkg-parsechangelog | sed -ne's,^Version:
\(.*\),\1,p')}
+true ${DEB_SUBST_VERSION:=$(echo $DEB_SUBST_PACKAGEVERSION | sed
-ne's,^\([0-9.]*\).*,\1,p')}
+true ${DEB_SUBST_DEBVERSION:=$(echo $DEB_SUBST_PACKAGEVERSION | awk -F '-' '{
print $NF }')}
+true ${DEB_SUBST_SVNPATH:=$(echo $DEB_SUBST_VERSION | awk -F '.' '{ print
"release_"$1"_"$2"_"$3 }')}
+true ${DEB_SUBST_UPSTREAM_VERSION:=$(echo ${DEB_SUBST_PACKAGEVERSION} | cut
-f 1 -d -)}
+true ${DEB_SUBST_UPSTREAM_MAIN_VERSION:=$(echo ${DEB_SUBST_UPSTREAM_VERSION}
| sed -e 's/^\([0-9\.]*\).*/\1/')}
+true ${DEB_SUBST_PACKAGESUFFIX:=-${DEB_SUBST_UPSTREAM_MAIN_VERSION}}
+true ${DEB_SUBST_PRIORITY:=$(($(echo ${DEB_SUBST_VERSION}.0.0.0.0 | sed -e
's@\([0-9]\)\+\.\([0-9]\)\+\.\([0-9]\+\)\.\([0-9]\+\).*@((\1*100+\2)*100+\3)*100+\4@')))}
+true ${DEB_SUBST_TARGET:=$(dpkg-architecture
-qDEB_BUILD_GNU_CPU)-$(dpkg-architecture -qDEB_BUILD_ARCH_OS)}
-DEB_SUBST_PACKAGEVERSION=`dpkg-parsechangelog | sed -ne's,^Version:
\(.*\),\1,p'`
-DEB_SUBST_VERSION=`echo $DEB_SUBST_PACKAGEVERSION | sed
-ne's,^\([0-9.]*\).*,\1,p'`
-DEB_SUBST_DEBVERSION=`echo $DEB_SUBST_PACKAGEVERSION | awk -F '-' '{ print $NF
}'`
-DEB_SUBST_SVNPATH=`echo $DEB_SUBST_VERSION | awk -F '.' '{ print
"release_"$1"_"$2"_"$3 }'`
-DEB_SUBST_TARGET="$2"
-DEB_SUBST_PPCBIN="$3"
-DEB_SUBST_PACKAGESUFFIX=${PACKAGESUFFIX}
-DEB_SUBST_PRIORITY=${4:-$(($(echo ${DEB_SUBST_VERSION}.0.0.0.0 | sed -e
's@\([0-9]\)\+\.\([0-9]\)\+\.\([0-9]\+\)\.\([0-9]\+\).*@((\1*100+\2)*100+\3)*100+\4@')))}
+if [ "${1}" = "--gen-control" ]
+then
+ gen_control=true
+ shift
+else
+ gen_control=false
+fi
+if test $# -lt 1
+then
+ usage
+fi
-SUBST_CMD=`set | sed -n -e 's/^DEB_SUBST_\([A-Z_]\+\)=\(.*\)/-e
s@\${\1}@\2@g/p'`
+echo 'List of defined variables'
+set | grep '^DEB_SUBST_'
+SUBST_CMD=$(set | sed -n -e 's/^DEB_SUBST_\([A-Z_]\+\)=\(.*\)/-e
s@\${\1}@\2@g/p')
-for i in $1/*.in
+for i in $*
do
- j=`basename ${i} .in`
- sed ${SUBST_CMD} ${i} > $1/${j/./${PACKAGESUFFIX}.}
+ f=$(basename ${i} .in)
+ if ${gen_control} || test ${f} != 'control'
+ then
+ d=$(dirname ${i})
+ sed ${SUBST_CMD} ${i} > ${d}/${f/./${DEB_SUBST_PACKAGESUFFIX}.}
+ fi
done
diff -Nru fpc-2.6.0/debian/rules fpc-2.6.0/debian/rules
--- fpc-2.6.0/debian/rules 2012-11-28 07:31:42.0 +0100
+++ fpc-2.6.0/debian/rules 2013-04-12 19
Bug#704252: preparing the fix
On Wed, 2013-04-10 at 23:20 +0200, Paul Gevers wrote: > The RT agrees with the patch for this issue. I have not discussed the > other items as I think they are not in line with the current freeze > exceptions. I can create an upload tomorrow evening. If you want you can > prepare that, via mentors, but that is not really needed. > > From IRC: > [23:08] Elbrus: it seems ok to me. the set -ex should become just > set -e though Committed Can you please take 2.6.0 branch head and upload. The other 2 items are just a typo in translation file and a fix suggested by release team themselves. I think we really conform to the freeze goals. However if you don't like these two fixes, please feel free to remove them. Thanks, signature.asc Description: This is a digitally signed message part
Bug#704252: preparing the fix
tags 704252 +patch pending thanks On 10-04-13 22:45, Abou Al Montacir wrote: > I've already committed this on svn on branch 2.6.0. I saw this after I sent the mail. > I'm attaching patches for info. If you agree I can upload to > mentors. The RT agrees with the patch for this issue. I have not discussed the other items as I think they are not in line with the current freeze exceptions. I can create an upload tomorrow evening. If you want you can prepare that, via mentors, but that is not really needed. From IRC: [23:08] Elbrus: it seems ok to me. the set -ex should become just set -e though Paul signature.asc Description: OpenPGP digital signature
