Ping! On Thu, Aug 4, 2016 at 8:03 AM, Adrian Prantl <apra...@apple.com> wrote: > >> On Aug 3, 2016, at 1:56 PM, Bruno Cardoso Lopes <bruno.card...@gmail.com> >> wrote: >> >> Hi Eric, >> >> After we upgraded our green dragon bots to El Captain (10.11), the >> test below started to fail on some of our machines: >> >> -- >> test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp >> >> TEST_CASE(hard_link_count_for_directory) >> { >> uintmax_t DirExpect = 3; >> uintmax_t Dir3Expect = 2; >> #if defined(__APPLE__) >> DirExpect += 2; >> Dir3Expect += 1; >> #endif > > Just as a general code review comment: When committing a platform-specific > workaround, I would expect there to be a comment explaining what > bug/peculiarity of the platform is being worked around :-) > > thanks, > Adrian > >> TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect); >> TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect); >> >> std::error_code ec; >> TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect); >> TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect); >> } >> >> You can see the issue in every recent (past 10 days) run on >> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/433 >> >> I noticed that commenting out the snippet around "#if >> defined(__APPLE__)" would make it work. What's the rationale behind >> the "#if defined(__APPLE__)" above? >> >> Thanks, >> >> >> On Tue, Jun 21, 2016 at 3:03 PM, Eric Fiselier via cfe-commits >> <cfe-commits@lists.llvm.org> wrote: >>> The issue should be fixed in r273323. >>> >>> Thanks for the report and for your patience. >>> >>> /Eric >>> >>> On Mon, Jun 20, 2016 at 11:27 PM, Eric Fiselier <e...@efcs.ca> wrote: >>>> >>>> Hi Artem, >>>> >>>> Sorry for the delay, I've been busy with school all day. >>>> I'll check in a fix tomorrow morning. >>>> >>>> Sorry again, >>>> >>>> /Eric >>>> >>>> On Mon, Jun 20, 2016 at 2:31 PM, Eric Fiselier <e...@efcs.ca> wrote: >>>>> >>>>> Oh shoot, I definitely didn't take that into account. I'll put together a >>>>> fix. >>>>> >>>>> /Eric >>>>> >>>>> >>>>> >>>>> On Mon, Jun 20, 2016 at 2:27 PM, Artem Belevich <t...@google.com> wrote: >>>>>> >>>>>> Eric, >>>>>> >>>>>> Some tests appear to fail if the path to the tests' current directory >>>>>> has some symlinks in it. >>>>>> In my case source and build tree are in directory 'work' that's >>>>>> symlinked to from my home directory: >>>>>> /usr/local/home/tra/work -> /work/tra >>>>>> >>>>>> This causes multiple failures in libcxx tests. One example: >>>>>> >>>>>> >>>>>> projects/libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.rename/rename.pass.cpp:121 >>>>>> 121 TEST_CHECK(read_symlink(bad_sym_dest) == dne); >>>>>> >>>>>> dne: >>>>>> >>>>>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne >>>>>> bad_sym_dest: >>>>>> >>>>>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2 >>>>>> >>>>>> These are the paths that traverse the 'work' symlink in my home >>>>>> directory. However, the symlink that we're reading contains physical >>>>>> path to >>>>>> the directory. While it does link to the right place, it's not the path >>>>>> the >>>>>> test expects. >>>>>> >>>>>> #readlink >>>>>> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2 >>>>>> >>>>>> /work/tra/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne >>>>>> >>>>>> I think we need to normalize paths before we compare them. >>>>>> >>>>>> --Artem >>>>>> >>>>>> >>>>>> On Sat, Jun 18, 2016 at 12:03 PM, Eric Fiselier via cfe-commits >>>>>> <cfe-commits@lists.llvm.org> wrote: >>>>>>> >>>>>>>> I assume the correct way to fix this is to disable >>>>>>>> -Wcovered-switch-default while compiling >>>>>>>> libcxx/src/experimental/filesystem/operations.cpp >>>>>>> >>>>>>> Agreed. Disabled in r273092. >>>>>>> >>>>>>> Thanks for your patience with this latest change, >>>>>>> >>>>>>> /Eric >>>>>>> >>>>>>> On Sat, Jun 18, 2016 at 12:54 PM, Adrian Prantl <apra...@apple.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> Hello Eric, >>>>>>>> >>>>>>>> this commit causes new warnings on our bots: >>>>>>>> >>>>>>>> clang/src/projects/libcxx/include/fstream:816:5: warning: default >>>>>>>> label in switch which covers all enumeration values >>>>>>>> [-Wcovered-switch-default] >>>>>>>> default: >>>>>>>> >>>>>>>> The problem is with this defensive default statement in fstream: >>>>>>>> >>>>>>>> >>>>>>>> template <class _CharT, class _Traits> >>>>>>>> 0792 typename basic_filebuf<_CharT, _Traits>::pos_type >>>>>>>> 0793 basic_filebuf<_CharT, _Traits>::seekoff(off_type __off, >>>>>>>> ios_base::seekdir __way, >>>>>>>> 0794 ios_base::openmode) >>>>>>>> 0795 { >>>>>>>> 0796 #ifndef _LIBCPP_NO_EXCEPTIONS >>>>>>>> 0797 if (!__cv_) >>>>>>>> 0798 throw bad_cast(); >>>>>>>> 0799 #endif >>>>>>>> 0800 int __width = __cv_->encoding(); >>>>>>>> 0801 if (__file_ == 0 || (__width <= 0 && __off != 0) || sync()) >>>>>>>> 0802 return pos_type(off_type(-1)); >>>>>>>> 0803 // __width > 0 || __off == 0 >>>>>>>> 0804 int __whence; >>>>>>>> 0805 switch (__way) >>>>>>>> 0806 { >>>>>>>> 0807 case ios_base::beg: >>>>>>>> 0808 __whence = SEEK_SET; >>>>>>>> 0809 break; >>>>>>>> 0810 case ios_base::cur: >>>>>>>> 0811 __whence = SEEK_CUR; >>>>>>>> 0812 break; >>>>>>>> 0813 case ios_base::end: >>>>>>>> 0814 __whence = SEEK_END; >>>>>>>> 0815 break; >>>>>>>> 0816 default: >>>>>>>> 0817 return pos_type(off_type(-1)); >>>>>>>> 0818 } >>>>>>>> >>>>>>>> >>>>>>>> I assume the correct way to fix this is to disable >>>>>>>> -Wcovered-switch-default while compiling >>>>>>>> libcxx/src/experimental/filesystem/operations.cpp >>>>>>>> >>>>>>>> Could you please investigate? >>>>>>>> >>>>>>>> thanks, >>>>>>>> Adrian >>>>>>> >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> cfe-commits mailing list >>>>>>> cfe-commits@lists.llvm.org >>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> --Artem Belevich >>>>> >>>>> >>>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> >> >> -- >> Bruno Cardoso Lopes >> http://www.brunocardoso.cc >
-- Bruno Cardoso Lopes http://www.brunocardoso.cc _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits