JavaScript is turned off

Review Board requires JavaScript in order to function. Please turn it on in your browser preferences.

Firefox users: if you prefer to turn on JavaScript only for specific sites, we recommend the NoScript extension.


Submitter:	Boroondas Gupte	Reviewers
Branch:		Groups:	viewer
Bugs:	OPEN-67	People:
Change Number:	None	Repository:	viewer-development

Description:

Context: We are currently using Boost 1.45, which already comes with the new Boost Filesystem Library API (Version 3) but still defaults to the old one (Version 2). From Boost 1.46 on, V3 will be the default and Boost 1.47 will be the last one to come with V2. The Boost Filesystem Library documentation recommends "Existing code should be moved to Version 3 as soon as convenient. New code should be written for Version 3. Version 2 is deprecated, and will not be included in Boost releases 1.48 and later."

This change overrides the default, so that the V3 API is used, and makes the necessary code changes. (So we can stick to Boost 1.45 and upgrade whenever we feel like it.)

Note: I only changed stuff that the compiler complained about. If the new API also changes semantic of still-compiling library usage, more changes might be necessary.

Testing Done:

* Compiled Viewer (standalone) with Boost 1.45
* Started Viewer
* Logged in

* Compiled Viewer (standalone) with Boost 1.46
* Started Viewer
* Logged in

Not tested:
* non-standalone

Expand All

work fine with both 1.45 from 3p-libs both custom 1.46 prebuilt libs (non-standalone)

Boroondas Gupte June 21, 2011, 3:14 a.m. (June 21, 2011, 3:14 a.m.)

Have you actually reviewed the code change or just tested the result?

indra/llvfs/lldiriterator.cpp (Diff revision 1)

What is your reasoning to use native() here and not string()?

Boroondas Gupte June 21, 2011, 7:19 a.m. (June 21, 2011, 7:19 a.m.)

I tried to stick to a 1:1 replacement. The old boost::filesystem::path::filename() has return type string_type, and boost::filesystem::path::native(), too, so that seemed like the natural replacement to me. As we save the result in a std::string, my reasoning might not make too much sense, though.

Aleric Inglewood June 21, 2011, 3:02 p.m. (June 21, 2011, 3:02 p.m.)

No that makes no sense at all, since boost::filesystem::path::native::string() also returns a std::string.
So, still the same question: why mIter->path().filename().native() and not mIter->path().filename().string()?

Aleric Inglewood June 21, 2011, 3:03 p.m. (June 21, 2011, 3:03 p.m.)
```
I meant boost::filesystem::path::string()
```

Boroondas Gupte June 21, 2011, 3:35 p.m. (June 21, 2011, 3:35 p.m.)

The reason I've given above is the only one I had. If that one doesn't make sense, no reason is left for preferring mIter->path().filename().native() over mIter->path().filename().string(), AFAIK. However, I also see no reason to prefer mIter->path().filename().string() over mIter->path().filename().native().

From the documentation, it seems they should give the exactly same data: the path's (here: the filename's) content as std::string with OS-native directory-separator characters (which shouldn't occur in a path that's just a filename). What could make sense is to use mIter->path().filename().generic_string(), so that the (effectless for pure filenames) '/' --> '\' conversion doesn't have to be executed on Windows.

Diff
- added Diff r2

Description:

Omit directory separator conversion, which is unnecessary for "paths" that are only filenames.

applied open-67_d.diff @r15984; windows fails link with unresolved externals as follows:
"windows-crash-logger.dir\RelWithDebInfo\/win_crash_logger.obj"  /machine:X86 /debug 
llvfs.lib(lldiriterator.obj) : error LNK2019: unresolved external symbol "private: static class std::codecvt<unsigned short,char,int> const * & __cdecl boost::filesystem3::path::wchar_t_codecvt_facet(void)" (?wchar_t_codecvt_facet@path@filesystem3@boost@@CAAAPBV?$codecvt@GDH@std@@XZ) referenced in function "public: __thiscall LLDirIterator::Impl::Impl(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &)" (??0Impl@LLDirIterator@@QAE@ABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@0@Z) [c:\Users\Bill\lindenhg\viewer-development-mesh\build-vc100\win_crash_logger\windows-crash-logger.vcxproj]
llvfs.lib(lldiriterator.obj) : error LNK2019: unresolved external symbol "void __cdecl boost::filesystem3::path_traits::convert(unsigned short const *,unsigned short const *,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > &,class std::codecvt<unsigned short,char,int> const &)" (?convert@path_traits@filesystem3@boost@@YAXPBG0AAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@ABV?$codecvt@GDH@5@@Z) referenced in function "public: class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const __thiscall boost::filesystem3::path::string(class std::codecvt<unsigned short,char,int> const &)const " (?string@path@filesystem3@boost@@QBE?BV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@ABV?$codecvt@GDH@5@@Z) [c:\Users\Bill\lindenhg\viewer-development-mesh\build-vc100\win_crash_logger\windows-crash-logger.vcxproj]
llvfs.lib(lldiriterator.obj) : error LNK2019: unresolved external symbol "public: class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const __thiscall boost::filesystem3::path::generic_string(class std::codecvt<unsigned short,char,int> const &)const " (?generic_string@path@filesystem3@boost@@QBE?BV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@ABV?$codecvt@GDH@5@@Z) referenced in function "public: class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const __thiscall boost::filesystem3::path::generic_string(void)const " (?generic_string@path@filesystem3@boost@@QBE?BV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@XZ) [c:\Users\Bill\lindenhg\viewer-development-mesh\build-vc100\win_crash_logger\windows-crash-logger.vcxproj]
llvfs.lib(lldiriterator.obj) : error LNK2019: unresolved external symbol "void __cdecl boost::filesystem3::path_traits::convert(char const *,char const *,class std::basic_string<unsigned short,struct std::char_traits<unsigned short>,class std::allocator<unsigned short> > &,class std::codecvt<unsigned short,char,int> const &)" (?convert@path_traits@filesystem3@boost@@YAXPBD0AAV?$basic_string@GU?$char_traits@G@std@@V?$allocator@G@2@@std@@ABV?$codecvt@GDH@5@@Z) referenced in function "void __cdecl boost::filesystem3::path_traits::dispatch<class std::basic_string<unsigned short,struct std::char_traits<unsigned short>,class std::allocator<unsigned short> > >(class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > const &,class std::basic_string<unsigned short,struct std::char_traits<unsigned short>,class std::allocator<unsigned short> > &,class std::codecvt<unsigned short,char,int> const &)" (??$dispatch@V?$basic_string@GU?$char_traits@G@std@@V?$allocator@G@2@@std@@@path_traits@filesystem3@boost@@YAXABV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@AAV?$basic_string@GU?$char_traits@G@std@@V?$allocator@G@2@@4@ABV?$codecvt@GDH@4@@Z) [c:\Users\Bill\lindenhg\viewer-development-mesh\build-vc100\win_crash_logger\windows-crash-logger.vcxproj]

Boroondas Gupte July 21, 2011, 1:37 p.m. (July 21, 2011, 1:37 p.m.)

Did the patch apply cleanly? To maintain readability, I didn't include the merge with tip, so you might want to pull https://bitbucket.org/boroondas/viewer-development-open-67 instead of manually applying patches.

Boroondas Gupte July 21, 2011, 3:58 p.m. (July 21, 2011, 3:58 p.m.)

Just saw that you mentioned a revision: 15984

Though, this is a revision number, thus valid for the local repository only. What is the corresponding revision ID?

Hash was 216ebc0442d29a8ba4a66ae8c7211b7aaa225892. However, the result is the same when building a clone of https://bitbucket.org/boroondas/viewer-development-open-67. I'll try to sort out a solution as time allows. Maybe another windows developer can pitch in.

https://bitbucket.org/lindenlab/viewer-development/changeset/a0b400b5ff0e/ The comments in this changeset describe the windows build / link issues for this CR.

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

OPEN-67: make LLDirIterator implementation compatible to boost::filesystem v3 (as found in Boost 1.44 and newer)