Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

Deal with alternating headers #212

Closed

Conversation

webmaster128
Copy link
Contributor

@webmaster128 webmaster128 commented Aug 22, 2016

As part of #179, alternating headers do not hit anymore. This is because in case of a changed header, a new manifest is created instead of updating the old one.

A (small) manifest looks like this

{
  "includeFiles": {
    "c:\\program files (x86)\\microsoft visual studio 14.0\\vc\\include\\concurrencysal.h": "cf7c28041ac710f2f4e8e03184ceb03a",
    "c:\\program files (x86)\\microsoft visual studio 14.0\\vc\\include\\sal.h": "25992979c3dc319b2b22111261d8b4ba",
    "c:\\program files (x86)\\microsoft visual studio 14.0\\vc\\include\\vadefs.h": "303c50a7bc924cd426baa20c7f16192c",
    "c:\\program files (x86)\\microsoft visual studio 14.0\\vc\\include\\vcruntime.h": "70f6d71b7615fc31611118aaa55f403d",
    "c:\\program files (x86)\\windows kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt.h": "c18832d918808d40b4664a51a8824a51",
    "c:\\program files (x86)\\windows kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_stdio_config.h": "035d8ef27e5026070e7650ea137714a0",
    "c:\\program files (x86)\\windows kits\\10\\include\\10.0.10240.0\\ucrt\\corecrt_wstdio.h": "2443db19dcc585e308f60dafef1d4c4c",
    "c:\\program files (x86)\\windows kits\\10\\include\\10.0.10240.0\\ucrt\\stdio.h": "2d5e699df1bed89fcccccf0dcfc49050"
  },
  "includesContentToObjectMap": {
    "b9e261c1d8ead6f14fe04c9e287abd1b": "20bde61f68b678191e2dff48321984a0"
  }
}

This manifest contains hashes of all includes as a map. In order to fix this, multiple file hashes per include must be allowed.

Given that there are no sets in JSON, includeFiles will need to become a dictionary from string to lists of strings.

As long as the include content equals one of the hashes, everything is fine and the manifest can be used. The actual include content is encoded in the includesContentToObjectMap key.

As soon as one include hash is not included, it becomes tricky: then we must deal with one manifest having multiple includes lists.

Dealing with multiple includes list per manifest is likely to be very painful. So I'll just start with a failing test.

@webmaster128
Copy link
Contributor Author

webmaster128 commented Aug 22, 2016

Interesting. This was not supposed to hit #155. But well, it does. Fine, let's do that first.

@frerich frerich changed the title [WIP] deal with alternating headers Deal with alternating headers Aug 24, 2016
@frerich frerich added bug wip and removed bug labels Aug 24, 2016
@siu
Copy link
Contributor

siu commented Aug 26, 2016

I started working in #222 to solve the race condition updating the manifests but along the way I found that it was easy to work on these issues too. I cherry-picked your unit tests there too.

@webmaster128
Copy link
Contributor Author

webmaster128 commented Aug 26, 2016

Sure, feel free. I did not start working on this yet. However, I don't think it is wise to use the test as long as #155 is not fixed, since the test currently fails for the wrong reason. But I did not dig into #222, so you might already have taken this into account.

@webmaster128
Copy link
Contributor Author

Closing this in favor of other people's solutions. I did not start working on an implementation yet.

@webmaster128 webmaster128 deleted the alternating-headers branch August 26, 2016 20:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants