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

[Zachary Turner via llvm-commits]

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.

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/223be00d/attachment.html>