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

Create new folder when packaging on linux #11983

Closed
cbrewster opened this issue Jul 1, 2016 · 14 comments
Closed

Create new folder when packaging on linux #11983

cbrewster opened this issue Jul 1, 2016 · 14 comments

Comments

@cbrewster
Copy link
Member

@cbrewster cbrewster commented Jul 1, 2016

Right now we take the target/[relase|debug] and remove everything but servo and bhtml and when we copy resources and add the run script. We should instead create a new temporary folder, copy servo, bhtml, resources, runscript to the new directory and create the tar.gz from that. After the tar.gz is created, the temporary folder should be deleted. This is similar to how packaging is done on the mac. The script should also check for the temporary folder when starting the packaging and delete it if it exists. If you don't do this, there will be errors with copying since shutil.copytree will not overwrite existing.

@highfive
Copy link

@highfive highfive commented Jul 1, 2016

Please make a comment here if you intend to work on this issue. Thank you!

@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 1, 2016

@ConnorGBrewster I would like to try solving this issue.

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Jul 1, 2016

It would be nice to do some locking of the packaging directory to prevent simultaneous builds, but I would not consider part of the E-easy (can be done in a follow-up).

@cbrewster cbrewster added the C-assigned label Jul 1, 2016
@cbrewster cbrewster mentioned this issue Jul 2, 2016
2 of 2 tasks complete
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 4, 2016

Not sure where to start. Would I be able to use Rust to write the code and implement @ConnorGBrewster's issue?

@cbrewster
Copy link
Member Author

@cbrewster cbrewster commented Jul 4, 2016

@Coder206 the packaging script is in python/servo/package_commands.py, if you look there there are android, OS X, and other branches for packaging. The other branch (which will be used for only linux in the future) currently makes changes to the [debug|release] folders when making the package. I would rather create a new directory and copy the needed files to the new directory than altering the [debug|release] folders. You can look at the mac branch as an example as it creates a new directory called dmg and copies the needed files to their respective locations there.

@aneeshusa aneeshusa added the L-python label Jul 4, 2016
@highfive
Copy link

@highfive highfive commented Jul 4, 2016

@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 4, 2016

@ConnorGBrewster Ok perfect. That makes sense! Thanks :)

@cbrewster
Copy link
Member Author

@cbrewster cbrewster commented Jul 15, 2016

@Coder206 any progress on this?

@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 15, 2016

@ConnorGBrewster Yes, I was beginning to understand how everything works. However, it will be at least a week before a PR is submitted. I understand if you want to give this to someone else to get it done quicker.

@cbrewster
Copy link
Member Author

@cbrewster cbrewster commented Jul 15, 2016

@Coder206 no worries, this isn't a huge priority right now.

@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 26, 2016

@ConnorGBrewster I made modification to python/servo/package_commands.py that reflect what you asked above based on OSX's code. I had a few questions:

  1. When you say copy "servo", should the script copy the whole directory?
  2. To test my code to make sure it works, is python package_commands.py the way to do that?
@jdm
Copy link
Member

@jdm jdm commented Jul 27, 2016

Copy servo means copy the servo binary.

@jdm
Copy link
Member

@jdm jdm commented Jul 27, 2016

I'm pretty sure the correct way to test it is ./mach package.

@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jul 27, 2016

@jdm Thanks

@Coder206 Coder206 mentioned this issue Aug 13, 2016
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Sep 30, 2016
New folder linux

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #11983 (github issue number if applicable).

- [x] There are tests for these changes (./mach package)

<!-- 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/12850)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Sep 30, 2016
New folder linux

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #11983 (github issue number if applicable).

- [x] There are tests for these changes (./mach package)

<!-- 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/12850)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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