Only remove the last extension from a source file name.

[ryan-phillips]

Fixes #78 by replacing change_extension() with add_extension(). Since get_basename() already removes the extension from a path expression, it isn't appropriate to change the extension of the base name.

All calls to change_extension() were made to an object returned by get_basename(), so there is no reason to keep both change_extension() and add_extension(), unless it is used by an external plugin.

[vslavik]

I don’t understand, neither from this nor from #78, what the actual problem is. Can you please describe a bit more explicitly, ideally if a sample bkl file demonstrating the problem (and really ideally, including the difference in generated output)?

Without knowing more, this doesn’t seem right, and if I’m reading this correctly, will uglify names of object files — I’d very much like to avoid that. If there’s some edge case that isn’t handled correctly, it would be better to fix that instead of changing the output this drastically. Again, if I’m understanding this patch correctly.

P.S. If you could use standard git commit message formatting, it would be greatly appreciated.

[ryan-phillips]

Sorry, I should have lead with an example. This commit only changes the output in a specific edge-case:

toolsets = gnu;

program test {
    sources {
        hello.x.c
    }
}

which currently outputs:

...
$(builddir)/test_hello.o: hello.x.c
...

while this is the desired output:

...
$(_builddir)test_hello.x.o: hello.x.c
...

The issue is in the way the object name is derived from the source file name. First, get_basename() removes the path and extension. Second, change_extension() removes the extension, if possible, and adds the object extension. However, in the common case it isn't possible to remove the extension in the second step because it was already removed in the first step. In effect, change_extension() is never used to change the extension as intended.

In short, only get_basename() or change_extension() should remove the extension, not both. I decided to modify change_extension() because it seemed to have the least impact. If you prefer to leave change_extension() alone, then get_basename() must not remove the extension.

[vslavik]

I’m terribly sorry, I was leaving for a trip right when you replied and by the time I came back, I completely forgot about it and GitHub’s notification mail felt under the fold :(

This commit only changes the output in a specific edge-case:

Are you sure, i.e. did you see if there are any changes in bkl’s test/example files?

The issue is in the way the object name is derived from the source file name. First, get_basename() removes the path and extension. Second, change_extension() removes the extension, if possible, and adds the object extension. However, in the common case it isn't possible to remove the extension in the second step because it was already removed in the first step. In effect, change_extension() is never used to change the extension as intended.

In short, only get_basename() or change_extension() should remove the extension, not both. I decided to modify change_extension() because it seemed to have the least impact. If you prefer to leave change_extension() alone, then get_basename() must not remove the extension.

I still don’t understand any of this. get_basename() of course must remove the extension, because that’s what it’s for — getting the basename (i.e. name sans extension). Similarly, change_extension() must change the extension… I also don’t understand what you changed it into _add__extension now and how it relates to the above explanation.

It also seems that you’re saying there’s a noop step somewhere and that there’s something removing too much, but I don’t see anything of the sorts in the commit’s diff — so again, what’s the relation to the changes?

Isn’t the actual problem something else, namely that both these functions consider too much to be an extension — i.e. that they treat “.x.c” as the extension in your example, not “.c”, or possibly only one of them (which?) does? If yes, then that is what needs fixing, the correct behavior obviously should be that get_basename("hello.x.c”) = “hello.x” and change_extension() replaces the “.c” part, not “.x.c”.