This repository was archived by the owner on Nov 17, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[MXNET-769] Usability improvements to windows builds #11947
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ | |
| from itertools import chain | ||
| from subprocess import call, check_call | ||
| from typing import * | ||
| from util import * | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this addition needed? Looks like you only removed from this file.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's needed for get_mxnet_root |
||
|
|
||
| CCACHE_MAXSIZE = '500G' | ||
|
|
||
|
|
@@ -138,24 +139,9 @@ def _get_local_image_id(docker_binary, docker_tag): | |
| return image_id | ||
|
|
||
|
|
||
| def get_mxnet_root() -> str: | ||
| curpath = os.path.abspath(os.path.dirname(__file__)) | ||
|
|
||
| def is_mxnet_root(path: str) -> bool: | ||
| return os.path.exists(os.path.join(path, ".mxnet_root")) | ||
|
|
||
| while not is_mxnet_root(curpath): | ||
| parent = os.path.abspath(os.path.join(curpath, os.pardir)) | ||
| if parent == curpath: | ||
| raise RuntimeError("Got to the root and couldn't find a parent folder with .mxnet_root") | ||
| curpath = parent | ||
| return curpath | ||
|
|
||
|
|
||
| def buildir() -> str: | ||
| return os.path.join(get_mxnet_root(), "build") | ||
|
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be two spaces between defs. |
||
| def default_ccache_dir() -> str: | ||
| # Share ccache across containers | ||
| if 'CCACHE_DIR' in os.environ: | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to bring it into the same format as our runtime functions on unix where we have all runtime functions (builds and tests) defined in that one file and it then goes into multiple branches? This allows us to be consistent in the methodology and you can see everything by looking at one file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know how to do this in powershell? it would take some time for marginal benefits right now. I'm open to do so in the future, but not inclined to do it further in this PR.