> 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits