MantisBT - Doomseeker
View Issue Details
0003234Doomseeker[All Projects] Bugpublic2017-09-01 11:452018-10-27 22:54
WubTheCaptain 
Blzut3 
normalmajorsometimes
closedfixed 
x86_64Debian GNU/Linuxbuster/sid
1.1 
1.21.2 
0003234: src/core/svnrevision.h may create "unexpected upstream changes" issues with dpkg-source
During the build process, directory listing of src/core/ may change unexpectedly and no longer matches upstream source archive. It is not cleaned up after build.

src/core/svnrevision.h is not included in source tar ball distribution. updaterevision tool autocreates this file during the build process.

Another possibility is svnrevision.h is removed from src/core/CMakeFileListing.txt during or after the build, causing "unexpected upstream changes" again.

This creates issues/burden on Debian package maintainers and reproducibility, requiring manual cleanup or patching for packaging Debian's .deb packages.

  1. Download Doomseeker 1.1 source fromhttp://doomseeker.drdteam.org/files/doomseeker-1.1_src.tar.bz2. [^]

  2. Extract it: tar xfvj doomseeker-1.1_src.tar.bz2

  3. Change directory to the extracted folder: cd doomseeker-1.1_src

  4. Create stub Debian packaging files for the next step (accepting single package and any details): dh_make -p doomseeker_1.1 -f ../doomseeker-1.1_src.tar.bz2

  5. Run debuild: debuild -us -uc

  6. Hit ^C (Ctrl+C) to cancel the build, or wait until the build is finished.

  7. Repeat previous debuild step.

  8. debuild fails to dpkg-source with error about "unexpected upstream changes".

doomseeker_1.1-1_amd64.build (failed build) results after first build attached.

"make clean" (or fakeroot ./debian/rules clean) doesn't clean this file, as far as I'm aware. It is also excluded in the .hgignore file.
No tags attached.
child of 0003246confirmed WubTheCaptain Debian packaging 
? doomseeker_1.1-1_amd64.build (1,058) 2017-09-01 11:45
https://zandronum.com/tracker/file_download.php?file_id=2181&type=bug
? doomseeker_1.1-1.diff.NLEjW2 (1,659) 2017-09-01 11:46
https://zandronum.com/tracker/file_download.php?file_id=2182&type=bug
log dpkg.log (712) 2017-09-01 11:48
https://zandronum.com/tracker/file_download.php?file_id=2183&type=bug
? doomseeker_1.2-1_amd64.build (1,018) 2018-09-24 22:33
https://zandronum.com/tracker/file_download.php?file_id=2412&type=bug
? doomseeker_1.2-1.diff.SXMVES (1,331) 2018-09-24 22:55
https://zandronum.com/tracker/file_download.php?file_id=2413&type=bug
? gitinfo.h (411) 2018-09-24 23:08
https://zandronum.com/tracker/file_download.php?file_id=2414&type=bug
Issue History
2017-09-01 11:45WubTheCaptainNew Issue
2017-09-01 11:45WubTheCaptainFile Added: doomseeker_1.1-1_amd64.build
2017-09-01 11:46WubTheCaptainNote Added: 0018206
2017-09-01 11:46WubTheCaptainFile Added: doomseeker_1.1-1.diff.NLEjW2
2017-09-01 11:46WubTheCaptainNote Deleted: 0018206
2017-09-01 11:48WubTheCaptainFile Added: dpkg.log
2017-09-01 16:29ZalewaRelationship addedchild of 0003246
2017-09-20 21:26ZalewaNote Added: 0018353
2017-09-20 21:26ZalewaAssigned To => Zalewa
2017-09-20 21:26ZalewaStatusnew => assigned
2017-09-24 16:11ZalewaNote Added: 0018365
2017-09-24 16:12ZalewaNote Edited: 0018365bug_revision_view_page.php?bugnote_id=18365#r10998
2017-09-24 16:12ZalewaNote Edited: 0018365bug_revision_view_page.php?bugnote_id=18365#r10999
2017-09-24 18:07WubTheCaptainNote Added: 0018373
2017-09-25 17:47WubTheCaptainNote Added: 0018387
2017-09-25 19:26ZalewaNote Added: 0018389
2017-09-25 19:27ZalewaNote Edited: 0018389bug_revision_view_page.php?bugnote_id=18389#r11009
2017-09-25 21:23ZalewaAssigned ToZalewa => Blzut3
2017-09-25 21:23ZalewaNote Added: 0018390
2017-12-17 17:04Blzut3Target Version => 1.2
2018-03-04 09:12Blzut3Note Added: 0019127
2018-03-04 09:12Blzut3Statusassigned => needs testing
2018-09-24 21:35WubTheCaptainNote Added: 0019718
2018-09-24 21:48ZalewaNote Added: 0019719
2018-09-24 22:33WubTheCaptainNote Added: 0019724
2018-09-24 22:33WubTheCaptainStatusneeds testing => needs review
2018-09-24 22:33WubTheCaptainFile Added: doomseeker_1.2-1_amd64.build
2018-09-24 22:34WubTheCaptainNote Edited: 0019724bug_revision_view_page.php?bugnote_id=19724#r11973
2018-09-24 22:35WubTheCaptainNote Edited: 0019724bug_revision_view_page.php?bugnote_id=19724#r11974
2018-09-24 22:38WubTheCaptainNote Added: 0019725
2018-09-24 22:38WubTheCaptainNote Edited: 0019725bug_revision_view_page.php?bugnote_id=19725#r11976
2018-09-24 22:40ZalewaNote Added: 0019726
2018-09-24 22:55WubTheCaptainNote Added: 0019727
2018-09-24 22:55WubTheCaptainFile Added: doomseeker_1.2-1.diff.SXMVES
2018-09-24 23:01WubTheCaptainNote Added: 0019728
2018-09-24 23:08WubTheCaptainFile Added: gitinfo.h
2018-09-25 00:46Blzut3Note Added: 0019731
2018-09-25 02:09WubTheCaptainStatusneeds review => needs testing
2018-09-25 02:12WubTheCaptainNote Added: 0019740
2018-09-25 02:21WubTheCaptainNote Added: 0019742
2018-09-25 02:21WubTheCaptainStatusneeds testing => resolved
2018-09-25 02:21WubTheCaptainFixed in Version => 1.2
2018-09-25 02:21WubTheCaptainResolutionopen => fixed
2018-09-29 15:06WubTheCaptainSeverityminor => major
2018-10-27 22:54WubTheCaptainStatusresolved => closed

Notes
(0018353)
Zalewa   
2017-09-20 21:26   
I think this file should be built in CMAKE_CURRENT_BINARY_DIR instead of src/core/. Doomseeker already looks for includes in that dir because that's where Qt tools build their files.
(0018365)
Zalewa   
2017-09-24 16:11   
(edited on: 2017-09-24 16:12)
I will have to retract the above statement as it would stand in contradiction with 3255 (reproducible builds).

Instead, svnrevision.h with properly filled information should be included in the source package and if 'updaterevision' tool fails to detect the repository it should not attempt to generate the file if that file already exists.

If the file doesn't exist and there's no repository to create it from, an invalid file should be created just as it is now.

(0018373)
WubTheCaptain   
2017-09-24 18:07   
At least the latter sounds like a sensible solution. Go for it.
(0018387)
WubTheCaptain   
2017-09-25 17:47   
Renaming the file to svnrevision.h.in and generating svnrevision.h from it may work.https://wiki.debian.org/UpstreamGuide#Cleaning_the_Tree [^]
(0018389)
Zalewa   
2017-09-25 19:26   
(edited on: 2017-09-25 19:27)
The svnrevision.h contains information that can be only generated from the repository. If you tarball the source code up and remove the .hg dir, then the information to generate svnrevision.h cannot be obtained anymore. The generator tool will create an invalid, yet compilable file instead (ie. revision info will be set to zero).

If we don't include the generated svnrevision.h in the archive, then how can we deterministically reproduce a build without the repository? The same page even suggests to use autorevision to export VCS metadata and include it in the tarball: https://wiki.debian.org/UpstreamGuide#Out-of-VCS_Builds. [^] This is exactly what is happening here, even though we use "our" own tool for that.

A template file seems to be hardly needed for a file that contains 3 #defines, none of which should be manually modified in any way.

As I see it, there are 2 problems that need to be solved here:

Problem 1. svnrevision.h file should not be touched if it already exists and it's impossible to determine the correct info from the repo. In such case, it will be necessary to assume that svnrevision.h already contains the correct information and touching it in any way will destroy it. Of course, people can just change other parts of the code and then compile Doomseeker without invalidating svnrevision.h, but given that we will already have reproducible builds, executables comparisons will expose their modifications.

Problem 2. svnrevision.h is currently not included in the tar.bz2 archive with the rest of the source code. When code archive is released, an already generated and correct for the given commit svnrevision.h should be included in the archive as well.

(0018390)
Zalewa   
2017-09-25 21:23   
Ad. 1. I have now checked and this is actually exactly what happens already:https://bitbucket.org/Doomseeker/doomseeker/src/2499674facfea585309bd0183de2bd270a253bb2/tools/updaterevision/updaterevision.c?at=default&fileviewer=file-view-default#updaterevision.c-135 [^]

So, this leaves us only with 2., hence I'm reassigning this to Blzut3.
(0019127)
Blzut3   
2018-03-04 09:12   
Rewrote our source package generation script to take this into account:https://bitbucket.org/Doomseeker/doomseeker/commits/f9a3f83d268484152f216b1592eeb68052e7abdf [^]
(0019718)
WubTheCaptain   
2018-09-24 21:35   
I've been largely ignoring makesourcepackages.sh exists in the repository, and have been testing against upstream tarballs available athttp://doomseeker.drdteam.org/files/ [^] so far.

Today I tried against makesourcepackages.sh. All good up until doomseeker-1.2/src/core/CMakeFileListing.cmake became the new violator, so I can't make a conclusive statement about the status of this specific bug. Since it's cmake related, perhaps a Debian packager only needs to adjust dh_clean target to clean up first.
(0019719)
Zalewa   
2018-09-24 21:48   
What is the meaning of "unexpected" in this context?
(0019724)
WubTheCaptain   
2018-09-24 22:33   
(edited on: 2018-09-24 22:35)
Quote from Zalewa
What is the meaning of "unexpected" in this context?


See attached doomseeker_1.1-1_amd64.build. It is the following error:

Quote from doomseeker_1.1-1_amd64.build
dpkg-source: error: aborting due to unexpected upstream changes, see /tmp/doomseeker_1.1-1.diff.NLEjW2


This means (meant) when building from inside the ./doomseeker-1.1 directory, the results of that "source code" (svnrevision.h and CMakeFileListing.txt) changed in that directory in comparison to ../doomseeker-1.1_src.orig.tar.bz2

It seems like the original build also knew about doomseeker-1.1_src/src/core/CMakeFileListing.txt (now doomseeker-1.2/src/core/CMakeFileListing.cmake) being modified, but OP didn't make a mention of it.

I did read doomseeker_1.2-1_amd64.build file generated by debuild from this compilation. It does seem like this ticket's issue with svnrevision.h is fixed; The issue with CMakeFileListing.cmake isn't fixed, and needs to be resolved in another ticket (or by the Debian package maintainer). I'll attach that file now.

The question is: Would someone create a ticket about CMakeFileListing.cmake having this same exact issue (even if the issue becomes invalid) and close this ticket as resolved? Thanks.

(0019725)
WubTheCaptain   
2018-09-24 22:38   
Quote from WubTheCaptain
The question is: Would someone create a ticket about CMakeFileListing.cmake having this same exact issue (even if the issue becomes invalid) and close this ticket as resolved? Thanks.


My request for you to close this ticket as resolved is kindly for your acknowledgement of the other issue, so that you won't run the steps to reproduce and think "oh it's still broken, this issue with svnrevision.h is not resolved". It's resolved. :)

(0019726)
Zalewa   
2018-09-24 22:40   
What does the diff say about the "unexpected changes" in CMakeFileListing.cmake in Doomseeker 1.2? You had one attached for problems in Doomseeker 1.1, I'd like to see the diff for 1.2 too.
(0019727)
WubTheCaptain   
2018-09-24 22:55   
Ah, I see. A related issue to this I guess.

--- doomseeker-1.2.orig/src/core/CMakeFileListing.cmake
+++ doomseeker-1.2/src/core/CMakeFileListing.cmake
@@ -74,6 +74,7 @@ set(HEADER_FILES
     filefilter.h
     fileutils.h
     gamedemo.h
+	gitinfo.h
     global.h
     gui/aboutdialog.h
     gui/commongui.h


Attaching the full file.
(0019728)
WubTheCaptain   
2018-09-24 23:01   
Bear in mind this is not a real and official Doomseeker 1.2 release but 1.2~beta-1 from source tree at commit a9995252fabcdd532cddcb763dc6c0931510e056 concerning the diff and amd64 build error with CMakeFileListing.cmake, despite the source directory name doomseeker-1.2. (Debian just expects them to be called that way by default, and makesourcepackages.sh makes doomseeker-1.2.tar.xz.)
(0019731)
Blzut3   
2018-09-25 00:46   
I think this should do it:https://bitbucket.org/Doomseeker/doomseeker/commits/644ac5daf69ff5f67a610d9c1976c99e4392d9b5 [^]
(0019740)
WubTheCaptain   
2018-09-25 02:12   
Blzut3: Fyi, it's not immediately obvious from your commit or the CMake comment lines why someone would do that or care about file listing differing.

I'll test this now, anyway. Results in a few.
(0019742)
WubTheCaptain   
2018-09-25 02:21   
Reproducible source with debuild and no problems with places where gitinfo.h is used (mainly, the "About Doomseeker" dialog).

Note: I didn't test compiling libwadseeker-1.2.tar.xz, only doomseeker-1.2.tar.xz.

Thanks!