On Sat, Jan 01, 2022 at 08:38:43PM -0500, Robert Middleton wrote: > The full path is already in the compiled code anyway, this would > simply add the ability to get the filename from the full path(so > another member to the LocationInfo class). I can see certain > circumstances where it is useful to have the full path, for example > when you have two files named the same(please don't do this). > > That does bring up a good point though, is that if you don't want > location info compiled in, we should have a preprocessor macro that > will compile out all of the location info data. We can already > compile out log messages of certain levels, so hiding the location is > just a natural extension of that.
Current log4cxx is reproducible [1] as in reproducible-builds.org, and one part of checking reproducibilty is to build in different location, it looks like that current log4cxx does NOT embed the complete path. So a wish from the Debian mainainer: It would be nice to retain reproduciblity when implementing this feature... A happy new year! -- tobi [1] https://tests.reproducible-builds.org/debian/rb-pkg/unstable/amd64/log4cxx.html (0.12.1 is currently not reproducible due to something doxygen is doing, but the library package is.) > -Robert Middleton > > On Sat, Jan 1, 2022 at 7:50 PM Stephen Webb <swebb2...@gmail.com> wrote: > > > > I prefer option 2 if it is possible. > > > > I do not think it is useful to have the full path name (of the build > > directory) in shipped binaries. > > > > On Sun, Jan 2, 2022 at 7:11 AM Robert Middleton <rmiddle...@apache.org> > > wrote: > > > > > PR #75 adds a new option for displaying the short filename of the log > > > location, which is probably a good idea to have, as in my experience, > > > the whole path of the file is not that useful, especially when the > > > binary is from a build server where the path is something like > > > /var/lib/jenkins/project-name/src/main/directory/foo.cpp. > > > > > > The current PR does this with some string manipulation at runtime, and > > > is different from the const char* that is currently used, so it > > > doesn't fit that well with the rest of the class. Since C++11 we can > > > now do constexpr functions, so we should be able to do this at compile > > > time(assuming you have optimizations turned on of course). > > > > > > We can do this one of several ways: > > > 1. Take the PR as is(use strings and calculate at runtime) > > > 2. Create a constexpr function that we control to calculate the > > > filename at compile time as either an offset into the filename or a > > > separate const char*. The advantage to this is that it doesn't > > > require any other libraries for pre-C++17 compilers. > > > 3. Use std::string_view(for C++17) or boost::string_view(pre-c++17). > > > The following would work for this solution: > > > std::string_view str{"/foo/bar/what.cpp"}; > > > const int location = str.find_last_of( '/' ) + 1; > > > printf( "fullPath: %s\n", str.data() ); > > > printf( "fileName: %s\n", &str[location] ); > > > > > > Does anybody have a preference or a better way to do it? I'm inclined > > > to go with (3), since that does provide a good fallback for compilers > > > that don't support C++17, and only requires boost for older compilers. > > > > > > -Robert Middleton > > >
