Opened 2 years ago

Closed 4 months ago

#1333 closed defect/bug (fixed)

XML files do not get re-generated reliably on build

Reported by: mvglasow (2) Owned by: KaZeR
Priority: major Milestone:
Component: tools Version: git master
Severity: normal Keywords: cmake, build, XML
Cc: http://wiki.navit-project.org/index.php/user:mvglasow, (2)

Description

When building with cmake, navit*.xml files seem to get generated only if they are not present in the build dir or if one of the C source files has changed. Changing one of the XSL files will not trigger a rebuild.

Steps to reproduce:

  • Build Navit for Android following the normal cmake build procedures.
  • Change one of the XSL files, e.g. osd_minimum.xsl.
  • Build again (do not clean the build dir).
  • Examine the XML files which were generated. The modification to the XSL file is not reflected.
  • Delete the XML file and build again.
  • Examine the XML files again. Now the change will be reflected.

Suggestion: unless we have logic to determine if one of the input files was changed (which might be complex), just generate the XML files from scratch on every build. (As far as I can tell, this is what is done for bitmaps. Generating the XML files is much quicker, so the performance penalty should be negligible.)

Change History (6)

comment:1 follow-up: Changed 2 years ago by mvglasow (2)

Just noticed a similar issue with Android bitmap assets in navit/android/res/drawable*. Here it's even worse, as deleting the files will cause the build to fail rather than the files to be re-copied. Modifying a bitmap means rebuilding everything from scratch – quite annoying when working on bitmap designs – or manually copying the bitmaps over, which is error-prone.

Again, this is a relatively small number of files, hence copying them on every build isn't going to carry a large performance penalty.

comment:2 Changed 2 years ago by sleske

We could also fix the dependency declarations. AFAIK, this is handled in (one of the) CMakeLists.txt. Maybe I can have a look...

comment:3 Changed 22 months ago by mvglasow (2)

  • Cc http://wiki.navit-project.org/index.php/user:mvglasow (2) added

comment:4 in reply to: ↑ 1 Changed 4 months ago by sleske

Replying to mvglasow (2):

Just noticed a similar issue with Android bitmap assets in navit/android/res/drawable*. Here it's even worse, as deleting the files will cause the build to fail rather than the files to be re-copied. Modifying a bitmap means rebuilding everything from scratch – quite annoying when working on bitmap designs – or manually copying the bitmaps over, which is error-prone.

I cannot reproduce that problem with the current version of Navit. Steps I tried:

  • run a complete build (make; make apkg): success
  • from the build directory: rm navit/android/res/drawable-hdpi/icon.png
  • re-run make; make apkg

Result: Successful build, and navit/android/res/drawable-hdpi/icon.png is correctly re-copied and included in the apkg.

So it seems this has been fixed.

comment:5 Changed 4 months ago by sleske

When building with cmake, navit*.xml files seem to get generated only if they are not present in the build dir or if one of the C source files has changed. Changing one of the XSL files will not trigger a rebuild.

Actually, changing the main XSLT file (android.xslt) will trigger a rebuild, as expected. The problem is that android.xslt includes other XSTL files, such as osd_minimum.xsl, via xsl:include. The build does not take that into account.

To solve this, the files navit*.xml are now regenerated when any XSLT file under xslt/ changes. That is a bit too generous, but better safe than sorry. Figuring out the real dependencies would require parsing the XSLT files for xsl:include and xsl:import recursively, which is tricky.

One other caveat: CMake will not pick up new files unter xslt/ automatically, so when you add a new file, you must re-run CMake manually. I added a README file explaining this.

comment:6 Changed 4 months ago by sleske

  • Resolution set to fixed
  • Status changed from new to closed

Fix committed as c5b3887.

Note: See TracTickets for help on using tickets.