Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added default fall-back when CARGO_HOME is not set for clean-cargo-cache #19843

Merged
merged 1 commit into from Jan 23, 2018

Conversation

@terracotaPie
Copy link
Contributor

terracotaPie commented Jan 23, 2018

Added default fall-back when CARGO_HOME is not set.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #19823 (github issue number if applicable).

This change is Reviewable

@highfive
Copy link

highfive commented Jan 23, 2018

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @mbrubeck (or someone else) soon.

@highfive
Copy link

highfive commented Jan 23, 2018

Heads up! This PR modifies the following files:

@terracotaPie
Copy link
Contributor Author

terracotaPie commented Jan 23, 2018

Let me know if I should fix anything :) Edit: it seems like some of the tests are failing, namely on windows:

WindowsError: [Error 3] The system cannot find the path specified: u'C:\\Users\\appveyor\\.cargo\\git\\db\\*.*'

Is it because .cargo location on windows is in the appdata?

More edit: I see that other PR's have similar issue

@terracotaPie
Copy link
Contributor Author

terracotaPie commented Jan 23, 2018

It seems after #19831 is fixed this should work fine. It seems jdm already proposed a pull request

@tigercosmos
Copy link
Collaborator

tigercosmos commented Jan 23, 2018

@terracotaPie Seems not work?

AppVeyor Log

mach clean-cargo-cache --keep 2 --force
Error running mach:
    ['clean-cargo-cache', '--keep', '2', '--force']
The error occurred in the implementation of the invoked mach command.
This should never occur and is likely a bug in the implementation of that
command. Consider filing a bug for this issue.
If filing a bug, please include the full output of mach, including this error
message.
The details of the failure are as follows:
WindowsError: [Error 3] The system cannot find the path specified: u'C:\\Users\\appveyor\\.cargo\\git\\db\\*.*'
  File "C:\projects\servo\python\servo\bootstrap_commands.py", line 239, in clean_cargo_cache
    git_db_list = filter(lambda f: not f.startswith('.'), os.listdir(git_db_dir))
Command exited with code 1
@terracotaPie
Copy link
Contributor Author

terracotaPie commented Jan 23, 2018

@tigercosmos other PRs have the same error. There is a fix which I will rebase as soon as it gets into master #19832

@tigercosmos
Copy link
Collaborator

tigercosmos commented Jan 23, 2018

@terracotaPie Ah! Sorry. I misunderstand that this PR can also fix that issue.

@terracotaPie
Copy link
Contributor Author

terracotaPie commented Jan 23, 2018

@tigercosmos No worries. I'll wait for the PR to be in master and then rebase. Would you mind reviewing then?

@jdm
Copy link
Member

jdm commented Jan 23, 2018

@bors-servo r+
This change will make it easier to experiment with other possible solutions for the appveyor situation.

@bors-servo
Copy link
Contributor

bors-servo commented Jan 23, 2018

📌 Commit a8ac8c8 has been approved by jdm

@highfive highfive assigned jdm and unassigned mbrubeck Jan 23, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jan 23, 2018

Testing commit a8ac8c8 with merge 06aa339...

bors-servo added a commit that referenced this pull request Jan 23, 2018
Added default fall-back when CARGO_HOME is not set for clean-cargo-cache

Added default fall-back when CARGO_HOME is not set.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #19823 (github issue number if applicable).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19843)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 23, 2018

@bors-servo bors-servo merged commit a8ac8c8 into servo:master Jan 23, 2018
2 of 4 checks passed
2 of 4 checks passed
Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.