Uploaded image for project: 'Qbs ("Cubes")'
  1. Qbs ("Cubes")
  2. QBS-128

FileInfo.baseName function is too greedy / non-standard

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P2: Important
    • Resolution: Done
    • Affects Version/s: 0.2
    • Fix Version/s: 0.2
    • Component/s: API: JavaScript
    • Labels:
      None

      Description

      Both the Javascript and C++ versions of the FileInfo:: baseName function are too greedy. Their current behaviour doesn't match any of the various definitions I found on the web, and furthermore prevents internally generated object file names from being correct!

      In particular, these functions are used internally by qbs to generate object filenames from source filenames, and the current implementation leads to broken behaviour.

      If there are files named thus:

      files: ["a.b.cpp","a.c.cpp"]

      the build fails, since both of these are converted to "a" (then "a.o") by the existing baseName function, so there's a clash in the object file names. The correct object filenames are "a.b.o" and "a.c.o".

      If they are to work like the function with the same name in GNU Make, they should remove everything after the /last/ dot. At the moment they remove everything after the /first/ dot in a filename, which is definitely wrong!

      Examples:

      Current (definitely broken) QBS behaviour: baseName ("x/y/a.b.c") => "a"
      GNU Make behaviour: $(basename "x/y/a.b.c") => "x/y/a.b"
      Arguably correct for QBS behaviour: baseName ("x/y/a.b.c") => "a.b"
      Correct according to common unix specification behaviour: baseName ("x/y/a.b.c") => "a.b.c", also baseName ("x/y/a.b.c",".c") => "a.b"

      The "arguably correct" version here is like a call to $(notdir $(basename "x/y/a.b.c")) in GNU Make or a call to “baseName "x/y/a.b.c" ".c"” in the single UNIX spec.

      References: http://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html , http://en.wikipedia.org/wiki/Basename

      The attached patches correct QBS's behaviour for both the C++ and Javascript implementations of this function to match the "Arguably correct".

      After some reading, though, it's worth noting that perhaps (to minimise confusion with other tools!) baseName should probably remove the directory part (as QBS already does) but only remove the suffix if explicitly passed that suffix (reference: http://en.wikipedia.org/wiki/Basename). In this way the behaviour would exactly match the POSIX behaviour.

      So the call should by that logic gain another parameter, and become baseName ("x/y/a.b.c",".c") => "a.b".

      I can see, however, that it's often more useful to /not know/ what the part after the last dot is, and to remove it (if it is not specified) regardless, and so behaviour like baseName ("x/y/a.b.c") => "a.b" is perhaps preferable.

      The attached patches, then, go for that "perhaps preferable" behaviour, without the extra optional suffix argument, … which would be straightforward to add if it's preferred! The attached patches make a minimal change to get things working for now.

      QBS is now working well for me here and avoiding the object file name clashes described above, after applying these two patches.

      Any preferences / suggestions?

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

            Assignee:
            jbornema Joerg Bornemann
            Reporter:
            gallafent William Gallafent
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Gerrit Reviews

                There are no open Gerrit changes