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

Replace handrolled build system with Thermite #88

Closed
wants to merge 6 commits into from

Conversation

malept
Copy link
Contributor

@malept malept commented Jul 4, 2016

Summary

Use thermite to help build and distribute faster_path.

This has several advantages to the current system:

Fixes #34, #71.

Next Steps

In order to use the GitHub releases feature of Thermite, I would suggest adding a GitHub releases deploy section to your Travis CI config. I have an example of this in my rusty_blank repository. That repository also has an AppVeyor CI config so that I can create binaries for Windows. (Although perhaps it might not be necessary - I haven't researched how feasible it is to cross-compile Rust extensions for Windows via Travis CI.)

If you want, I can add the deploy section (sans deploy key, obviously) to the Travis CI config.

@danielpclark
Copy link
Owner

I've opened an issue for verifying thermite works with tests and to help make sure it won't become broken in the future.

@malept malept mentioned this pull request Nov 6, 2016
@malept
Copy link
Contributor Author

malept commented Dec 10, 2016

For the record, this pull request should be good to go. Let me know if there's anything that needs to be changed or requires further discussion.

@danielpclark
Copy link
Owner

danielpclark commented Dec 11, 2016

Could you educate on me on how the binary build and the hosting of binaries for each architecture is achieved? I guess I get the concept but haven't fully grokked the how.

  • Where are the binaries built?
  • Where are they stored?
  • How are they trusted?

I see it has an option for Github binary releases.

  • Does this put the binaries in the origin/master branch of the Github repo?
  • Could I keep the binaries separate for the sanity of downloading from git source?

Perhaps some of this is covered in your NEXT STEPS section

@malept
Copy link
Contributor Author

malept commented Dec 11, 2016

Where are the binaries built?

Via your CI service(s). For me, Travis (Linux/OSX) and AppVeyor (Windows).

Where are they stored?

The way I've written this pull request, GitHub Releases, in your case https://github.com/danielpclark/faster_path/releases. In theory you could instead deploy to someplace like bintray or S3.

How are they trusted?

Trust with CI is Hard:tm:. I do have an issue open and some code partway written to add support for checksums, but that's only a part of the way when you're dealing with Trust.

Does this put the binaries in the origin/master branch of the Github repo?
Could I keep the binaries separate for the sanity of downloading from git source?

Thermite does not commit Rust binaries into a git repository. I personally don't put binaries anywhere near my git repositories unless I have to (and even then, I would evaluate git-lfs first).

@stereobooster
Copy link
Contributor

Any news on that? Do you need help?

@danielpclark
Copy link
Owner

danielpclark commented Feb 28, 2017

I'm ready to move forward with this @stereobooster & @malept . I've posted a question about cross compilation here: malept/thermite#32

Although cross compilation can come later.

@malept
Copy link
Contributor Author

malept commented Feb 28, 2017

I will be happy to rebase this PR.

@danielpclark Are checksum and/or cross compilation support the only blockers?

@danielpclark
Copy link
Owner

Yes I believe so. But I'm cool with getting this going now as is and we can build on it.

@malept malept force-pushed the integrate-thermite branch 2 times, most recently from e1125ee to 845d809 Compare March 1, 2017 07:44
@coveralls
Copy link

coveralls commented Mar 1, 2017

Coverage Status

Coverage decreased (-53.5%) to 38.71% when pulling 845d809 on malept:integrate-thermite into 6afbf2e on danielpclark:master.

@danielpclark
Copy link
Owner

danielpclark commented Mar 1, 2017

I have written a just-in-case bit of code that would help people who gem installed and didn't get the compiled asset: https://github.com/danielpclark/faster_path/blob/master/lib/faster_path/asset_resolution.rb . Its second purpose is to raise an error with a clear message as to why the require can't be done.

You may need to update this to point to any changes in location of the built library.

@danielpclark danielpclark added this to the 0.2.0 milestone Mar 1, 2017
@malept
Copy link
Contributor Author

malept commented Mar 1, 2017

That...is a lot of nesting. I'll try to find some time to adapt that.

@danielpclark
Copy link
Owner

danielpclark commented Mar 1, 2017

That...is a lot of nesting.

Haha! I'll refactor it right now to limit the nesting. ... and done!

@danielpclark
Copy link
Owner

If you find the time to get this done soon (say maybe this weekend) I'd love to contribute to Thermite as well to help with cross compilation. My mind is gearing up for it as I've been thinking heavily in this area.

@malept
Copy link
Contributor Author

malept commented Mar 4, 2017

It might happen Sunday, we'll see. I would be happy to have help with the cross compilation feature.

@malept malept force-pushed the integrate-thermite branch from 845d809 to 061ad19 Compare March 6, 2017 01:23
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 061ad19 on malept:integrate-thermite into ** on danielpclark:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling a5e293d on malept:integrate-thermite into ** on danielpclark:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8229ea3 on malept:integrate-thermite into ** on danielpclark:master**.

@malept malept force-pushed the integrate-thermite branch from 9094485 to 141656f Compare March 8, 2017 15:41
@danielpclark
Copy link
Owner

I believe my asset resolution code may have caused problems, this PR included. After I write the next two methods for this library I'll take the work you've done here in this PR and apply it to FasterPath.

If I have any questions that come up are you online enough that I might be able to get answers @malept ?

@malept
Copy link
Contributor Author

malept commented Feb 27, 2018

Less so in the next couple of days, but after that I should be able to answer questions (or at least say that I'd need to do research) at a reasonable pace.

@danielpclark
Copy link
Owner

Moved to #141. Working on the Github releases part now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants