Skip to content

/ servo

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

auto clean cache in build bot #19713

Merged
merged 1 commit into from Jan 18, 2018
Merged

auto clean cache in build bot #19713

merged 1 commit into from Jan 18, 2018

Conversation

@tigercosmos
Copy link
Collaborator

tigercosmos commented Jan 7, 2018 •
edited by SimonSapin

auto clean cache in build bot

once this merged, servo/saltfs#321 should be closed

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #19712 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable
@highfive
Copy link

highfive commented Jan 7, 2018

Heads up! This PR modifies the following files:
@aneeshusa
Copy link
Member

aneeshusa commented Jan 7, 2018

@bors-servo r+
Thanks for finishing this off!
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

📌 Commit c67d6fb has been approved by aneeshusa
@highfive highfive assigned aneeshusa and unassigned nox Jan 7, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

⌛ Testing commit c67d6fb with merge 476f28e...
bors-servo added a commit that referenced this pull request Jan 7, 2018
@bors-servo
Auto merge of #19713 - tigercosmos:r1, r=aneeshusa
Loading status checks…
476f28e 
auto clean cache in build bot

<!-- Please describe your changes on the following line: -->
auto clean cache in build bot

once this merged, servo/saltfs#321 should be closed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #19712  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/19713)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

💔 Test failed - mac-rel-wpt4
@KiChjang
Copy link
Member

KiChjang commented Jan 7, 2018 

Error running mach:

    ['clean-cargo-cache', '--keep', '3', '--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:

OSError: [Errno 2] No such file or directory: '/Users/servo/buildbot/slave/mac-rel-wpt4/build/.cargo/git/db'

  File "/Users/servo/buildbot/slave/mac-rel-wpt4/build/python/servo/bootstrap_commands.py", line 417, in clean_cargo_cache
    git_db_list = filter(lambda f: not f.startswith('.'), os.listdir(git_db_dir))
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 8, 2018 •
edited

All build machines failed with this message. I cannot imagine why since local can run the script.
CIs are also correct with the script.
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 8, 2018

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2018

⌛ Trying commit f218520 with merge 18cffa7...
bors-servo added a commit that referenced this pull request Jan 8, 2018
@bors-servo
Auto merge of #19713 - tigercosmos:r1, r=<try>
Loading status checks…
18cffa7 
auto clean cache in build bot

<!-- Please describe your changes on the following line: -->
auto clean cache in build bot

once this merged, servo/saltfs#321 should be closed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #19712  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/19713)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 8, 2018

💔 Test failed - mac-rel-wpt1
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 8, 2018 •
edited

  • Windows:
WindowsError: [Error 5] Access is denied: u'C:\\Users\\Administrator\\.cargo\\git\\checkouts\\android-rs-injected-glue-2d7b4c538126339e\\1995be2\\.git\\objects\\pack\\pack-d8250a700300a81c3e2d1109a18324a3c3033b6f.idx'

  File "c:\buildbot\slave\windows-msvc-dev\build\python\servo\bootstrap_commands.py", line 496, in clean_cargo_cache
    delete(crate_path)
  File "c:\buildbot\slave\windows-msvc-dev\build\python\servo\util.py", line 41, in delete
    shutil.rmtree(path)
  File "C:\Python27\lib\shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "C:\Python27\lib\shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "C:\Python27\lib\shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "C:\Python27\lib\shutil.py", line 252, in rmtree
    onerror(os.remove, fullname, sys.exc_info())
  File "C:\Python27\lib\shutil.py", line 250, in rmtree
    os.remove(fullname)

@aneeshusa Windows's cargo is under admin, seems we cannot touch it.

  • Android, linux-rel-nogate, linux-rel-css , arm64, arm32
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1: ordinal not in range(128)

  File "/home/servo/buildbot/slave/android/build/python/servo/bootstrap_commands.py", line 496, in clean_cargo_cache
    delete(crate_path)
  File "/home/servo/buildbot/slave/android/build/python/servo/util.py", line 41, in delete
    shutil.rmtree(path)
  File "/usr/lib/python2.7/shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "/usr/lib/python2.7/shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "/usr/lib/python2.7/shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "/usr/lib/python2.7/shutil.py", line 247, in rmtree
    rmtree(fullname, ignore_errors, onerror)
  File "/usr/lib/python2.7/shutil.py", line 241, in rmtree
    fullname = os.path.join(path, name)
  File "/usr/lib/python2.7/posixpath.py", line 80, in join
    path += '/' + b
@aneeshusa
Copy link
Member

aneeshusa commented Jan 8, 2018

@bors-servo r-

I suspect there is an actual error here as the error message indicates that ./mach clean-cargo-cache is not respecting the SERVO_CACHE_DIR we have set on the builders: https://github.com/servo/saltfs/blob/36dbe2c360abf2858921914a9ae01aa4eef17992/buildbot/master/files/config/environments.py#L72-L88.

My guess is that recent changes to the bootstrapping code may have caused this; cc @SimonSapin who has touched this recently.
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 8, 2018

@aneeshusa the origin issue can solve by adding --custom-path to find the cargo home.

but there is new issue as I post above
@aneeshusa
Copy link
Member

aneeshusa commented Jan 8, 2018

@tigercosmos ah sorry about that, I commented at the same time as you and didn't see your comment! It looks like there are some more issues with ./mach clean-cargo-cache that need to be fixed.

I'm not sure about what to do about the first issue with access denied, but the second issue looks like bytes/unicode confusion that will need to be fixed.

Also, good catch that this is CARGO_HOME and not SERVO_CACHE_DIR; IMO we should get rid of the --custom-path flag and just always use that behavior.
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 8, 2018

@aneeshusa I don't understand what you mean "always use that behavior"?
Do you mean we should remove the flag --custom-path? But the config defines a new dir to save cache.
@tigercosmos tigercosmos mentioned this pull request Jan 15, 2018
Closed
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 16, 2018

could someone review?
@jdm
jdm requested changes Jan 16, 2018
View changes
etc/ci/buildbot_steps.yml Outdated
@@ -156,6 +176,7 @@ windows-msvc-dev:
- mach.bat test-stylo

windows-msvc-nightly:
- mach.bat clean-cargo-cache --keep 3 --force --custom-path

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Jan 16, 2018

Member

Since #19395 has merged, let's make the --custom-path behaviour the default behaviour and remove the argument entirely from the mach command implementation.

This comment has been minimized.

Sign in to view
Copy link
@tigercosmos

tigercosmos Jan 18, 2018

Author Collaborator

@jdm Done
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 18, 2018

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 18, 2018

⌛ Trying commit 8e8a9ce with merge 8d8fdbd...
bors-servo added a commit that referenced this pull request Jan 18, 2018
@bors-servo
Auto merge of #19713 - tigercosmos:r1, r=<try>
Loading status checks…
8d8fdbd 
auto clean cache in build bot

<!-- Please describe your changes on the following line: -->
auto clean cache in build bot

once this merged, servo/saltfs#321 should be closed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #19712  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/19713)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 18, 2018

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True
@tigercosmos
auto clean cache
Loading status checks…
ebf8348
@tigercosmos
Copy link
Collaborator Author

tigercosmos commented Jan 18, 2018

r? @jdm
@highfive highfive assigned jdm and unassigned aneeshusa Jan 18, 2018
@jdm
Copy link
Member

jdm commented Jan 18, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jan 18, 2018

📌 Commit ebf8348 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 18, 2018

⌛ Testing commit ebf8348 with merge d11886e...
bors-servo added a commit that referenced this pull request Jan 18, 2018
@bors-servo
Auto merge of #19713 - tigercosmos:r1, r=jdm
Loading status checks…
d11886e 
auto clean cache in build bot

<!-- Please describe your changes on the following line: -->
auto clean cache in build bot

once this merged, servo/saltfs#321 should be closed

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #19712  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/19713)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 18, 2018

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing d11886e to master...
@bors-servo bors-servo merged commit ebf8348 into servo:master Jan 18, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Jan 18, 2018
Merged
2 of 2 tasks complete
@tigercosmos tigercosmos deleted the tigercosmos:r1 branch Jan 18, 2018
@jdm jdm mentioned this pull request Mar 7, 2018
Closed
bors-servo added a commit that referenced this pull request Mar 8, 2018
@bors-servo
Auto merge of #20233 - servo:jdm-patch-9, r=emilio
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ad34efa 
Don't clean the cargo cache on Windows.

I previously added the CARGO_HOME for Windows builders because I thought there might be a permissions problem with the default location that was causing the errors when cloning repositories. I suspect that cleaning the cache is to blame for those somehow, since as far as I recall the problems started after #19713. Let's take it out and see if they go away!

<!-- 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/20233)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Mar 8, 2018
@bors-servo
Auto merge of #20233 - servo:jdm-patch-9, r=emilio
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
3dfc607 
Don't clean the cargo cache on Windows.

I previously added the CARGO_HOME for Windows builders because I thought there might be a permissions problem with the default location that was causing the errors when cloning repositories. I suspect that cleaning the cache is to blame for those somehow, since as far as I recall the problems started after #19713. Let's take it out and see if they go away!

<!-- 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/20233)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm

Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

auto clean cargo cache
8 participants
@tigercosmos @highfive @aneeshusa @bors-servo @KiChjang @SimonSapin @jdm @nox
You can’t perform that action at this time.