Skip to content

Conversation

@jcbrill
Copy link

@jcbrill jcbrill commented May 15, 2022

Change msvc cache key from string to tuple.

Convert cache dictionary to a list of dictionaries with key tuple converted to key list when writing cache json file.
Convert list of dictionaries with key list converted to key tuple to the cache dictionary when reading cache json file.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt (and read the README.rst)
  • I have updated the appropriate documentation

@jcbrill
Copy link
Author

jcbrill commented May 15, 2022

Feel free to ignore.

Per request:

Is the tuple-ification of the key planned?
@jcbrill could you share the changes If my count is correct, it would take approximately two new lines of code and changes to three lines of code. ?

@mwichmann Do you see anything of concern?

Note: there is an opportunity to increase the efficacy of the cache by normalizing the script path and case on Windows as discussed in SCons#4111.

p = Path(CONFIG_CACHE)
with p.open('r') as f:
envcache = json.load(f)
envcache_list = json.load(f)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine, I'm inclined to merge this change. Maybe a comment why we're doing what we're doing with came out of the json file (and/or what goes into it).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better or worse? See below as well.

@mwichmann mwichmann merged commit c816be1 into mwichmann:msvc/cachefix May 15, 2022
@jcbrill jcbrill deleted the mwichmann-msvc-cachefix branch May 15, 2022 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants