[llvm] r307426 - [llvm-pdbutil] Fix build.

[David Blaikie via llvm-commits]

On Mon, Jul 10, 2017 at 11:23 AM Zachary Turner <zturner at google.com> wrote:

> Ahh, you are right.  Looking over this change, I'm not sure what the LLD
> test change even is.  I'm guessing it's a line ending change (probably
> accidentally committed as CRLF and then fixed it to LF in the "build fix"
> patch.  Which if so, you're right, that should have been done separately.
>

Ah, yeah, I couldn't really spot the difference but figured it was related
to "Also a test involving case sensitivity needed to be fixed.  For now
that particular check just accepts any path even if they're completely
different." from the commit message in some way.


>
> On Mon, Jul 10, 2017 at 11:07 AM David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Mon, Jul 10, 2017 at 10:57 AM Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> Doesn't that defeat the entire point of the mono-repo though?
>>>
>>
>> No - this advise already exists even when committing two independent
>> changes in a single repo in LLVM. Independent changes should generally be
>> committed independently - makes it easier to understand them through the
>> commit history, to revert/reapply if needed, etc.
>>
>>
>>>   Committing them separately in the current mono-repo is a lot of extra
>>> work, which people have already spent a lot of time writing tooling for to
>>> make it as easy as possible to commit everything "all at once" (which leads
>>> to this scenario).  Going back on that seems contrary to the direction of
>>> the mono-repo.
>>>
>>> I think it would be all the more reason to finalize the transition to
>>> the mono-repo more quickly so that they're not actually separate repos on
>>> the backend and then they show up as one commit.
>>>
>>> On Mon, Jul 10, 2017 at 10:52 AM David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>> On Mon, Jul 10, 2017 at 10:51 AM Zachary Turner <zturner at google.com>
>>>> wrote:
>>>>
>>>>> Ahh, I'm using the mono-repo.  When I do one cross-repo commit it
>>>>> creates one commit in each repo with an identical message.  Not sure if
>>>>> there's a better solution,
>>>>>
>>>>
>>>> All the more reason to commit independent changes independently, I
>>>> would think
>>>>
>>>>
>>>>> seems like more and more people are switching to mono-repo for daily
>>>>> workflow.
>>>>>
>>>>> On Mon, Jul 10, 2017 at 10:48 AM David Blaikie <dblaikie at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Ah, this was a build break fix?
>>>>>>
>>>>>> Perhaps I've been confused by the cross-repo commit. Two changes with
>>>>>> the same change description, fixing two independent things in two different
>>>>>> repos... I thought they were related (that the fix in LLVM was
>>>>>> fixing/changing the test output in the lld test), but I guess they weren't?
>>>>>>
>>>>>> Might be better to commit such things separately, with
>>>>>> separate/specific descriptions.
>>>>>>
>>>>>> On Mon, Jul 10, 2017 at 10:38 AM Zachary Turner <zturner at google.com>
>>>>>> wrote:
>>>>>>
>>>>>>> A test for what, specifically?
>>>>>>>
>>>>>>> On Mon, Jul 10, 2017 at 10:20 AM David Blaikie <dblaikie at gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Looks like there should be a test for this in the LLVM tree, if
>>>>>>>> that's where the functionality is (with a committed PDB file or similar (if
>>>>>>>> there's a convenient YAML format or something, that's OK too), as
>>>>>>>> llvm-dwarfdump is tested)
>>>>>>>>
>>>>>>>> On Fri, Jul 7, 2017 at 12:00 PM Zachary Turner via llvm-commits <
>>>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>>>
>>>>>>>>> Author: zturner
>>>>>>>>> Date: Fri Jul  7 12:00:06 2017
>>>>>>>>> New Revision: 307426
>>>>>>>>>
>>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=307426&view=rev
>>>>>>>>> Log:
>>>>>>>>> [llvm-pdbutil] Fix build.
>>>>>>>>>
>>>>>>>>> Some platforms require an explicit specialization of std::hash
>>>>>>>>> for PdbRaw_FeaturesSig.  Also a test involving case sensitivity
>>>>>>>>> needed to be fixed.  For now that particular check just accepts
>>>>>>>>> any path even if they're completely different.  Long term we
>>>>>>>>> should output paths in the correct case to match MSVC.
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>>     llvm/trunk/tools/llvm-pdbutil/DiffPrinter.h
>>>>>>>>>
>>>>>>>>> Modified: llvm/trunk/tools/llvm-pdbutil/DiffPrinter.h
>>>>>>>>> URL:
>>>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-pdbutil/DiffPrinter.h?rev=307426&r1=307425&r2=307426&view=diff
>>>>>>>>>
>>>>>>>>> ==============================================================================
>>>>>>>>> --- llvm/trunk/tools/llvm-pdbutil/DiffPrinter.h (original)
>>>>>>>>> +++ llvm/trunk/tools/llvm-pdbutil/DiffPrinter.h Fri Jul  7
>>>>>>>>> 12:00:06 2017
>>>>>>>>> @@ -13,12 +13,23 @@
>>>>>>>>>  #include "llvm/ADT/ArrayRef.h"
>>>>>>>>>  #include "llvm/ADT/StringMap.h"
>>>>>>>>>  #include "llvm/ADT/StringRef.h"
>>>>>>>>> +#include "llvm/DebugInfo/PDB/Native/RawConstants.h"
>>>>>>>>>  #include "llvm/Support/FormatVariadic.h"
>>>>>>>>>  #include "llvm/Support/raw_ostream.h"
>>>>>>>>>
>>>>>>>>>  #include <list>
>>>>>>>>>  #include <unordered_set>
>>>>>>>>>
>>>>>>>>> +namespace std {
>>>>>>>>> +template <> struct hash<llvm::pdb::PdbRaw_FeatureSig> {
>>>>>>>>> +  typedef llvm::pdb::PdbRaw_FeatureSig argument_type;
>>>>>>>>> +  typedef std::size_t result_type;
>>>>>>>>> +  result_type operator()(argument_type Item) const {
>>>>>>>>> +    return std::hash<uint32_t>{}(uint32_t(Item));
>>>>>>>>> +  }
>>>>>>>>> +};
>>>>>>>>> +} // namespace std
>>>>>>>>> +
>>>>>>>>>  namespace llvm {
>>>>>>>>>  namespace pdb {
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> llvm-commits mailing list
>>>>>>>>> llvm-commits at lists.llvm.org
>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>>>>
>>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/87aa3bc1/attachment.html>