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

Update AppVeyor config. #5240

Merged
merged 2 commits into from Aug 29, 2016

Conversation

Projects
None yet
4 participants
@XhmikosR
Contributor

XhmikosR commented Aug 13, 2016

Install gems in vendor and cache those only.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 13, 2016

Contributor

Hmm, so it seems this can't work without a Gemfile.lock. Too bad, because the current solution to cache the whole bin folder is bad.

I guess I will just remove caching...

Contributor

XhmikosR commented Aug 13, 2016

Hmm, so it seems this can't work without a Gemfile.lock. Too bad, because the current solution to cache the whole bin folder is bad.

I guess I will just remove caching...

Show outdated Hide outdated appveyor.yml
# If one of the files after the right arrow changes, cache will be skipped
- C:\Ruby%RUBY_FOLDER_VER%\bin -> Gemfile,jekyll.gemspec,appveyor.yml
- C:\Ruby%RUBY_FOLDER_VER%\lib\ruby\gems\%GEMS_FOLDER_VER% -> Gemfile,jekyll.gemspec,appveyor.yml
# - 'vendor -> appveyor.yml,Gemfile,jekyll.gemspec'

This comment has been minimized.

@parkr

parkr Aug 22, 2016

Member

Are we then not caching vendor? Or are we relying on the automatic inclusion of this directory?

@parkr

parkr Aug 22, 2016

Member

Are we then not caching vendor? Or are we relying on the automatic inclusion of this directory?

This comment has been minimized.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

We can't use a custom deployment location because bundler complains that there is no Gemfile.lock. See https://ci.appveyor.com/project/jekyll/jekyll/build/259

I'm no expert with Rubygems so perhaps there is another way to do it instead of using bundler --deployment.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

We can't use a custom deployment location because bundler complains that there is no Gemfile.lock. See https://ci.appveyor.com/project/jekyll/jekyll/build/259

I'm no expert with Rubygems so perhaps there is another way to do it instead of using bundler --deployment.

This comment has been minimized.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I can go back to the previous solution but to me caching the whole bin folder seems bad. Or the system wide gems folder...

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I can go back to the previous solution but to me caching the whole bin folder seems bad. Or the system wide gems folder...

This comment has been minimized.

@parkr

parkr Aug 22, 2016

Member

Haha yeah caching the system-wide gems folder is not ideal. You shouldn't need --deployment for us (that's for running bundle install before you boot a server and it's why heroku, for example, requires the Gemfile.lock) – can we just do a custom list of --without to make sure we only install what we need?

@parkr

parkr Aug 22, 2016

Member

Haha yeah caching the system-wide gems folder is not ideal. You shouldn't need --deployment for us (that's for running bundle install before you boot a server and it's why heroku, for example, requires the Gemfile.lock) – can we just do a custom list of --without to make sure we only install what we need?

This comment has been minimized.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I'm reading http://bundler.io/v1.12/bundle_install.html and deployment seems the case for us.

We already have without https://github.com/jekyll/jekyll/blob/master/appveyor.yml#L19

So basically, we need to instruct bundle to install the gems in a custom folder which we can cache.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I'm reading http://bundler.io/v1.12/bundle_install.html and deployment seems the case for us.

We already have without https://github.com/jekyll/jekyll/blob/master/appveyor.yml#L19

So basically, we need to instruct bundle to install the gems in a custom folder which we can cache.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 22, 2016

Member

Thank you, @XhmikosR!

Member

parkr commented Aug 22, 2016

Thank you, @XhmikosR!

Show outdated Hide outdated appveyor.yml
# Bundler downgrade should be removed when the issue is fixed upstream
# - gem uninstall bundler -x
# - gem install bundler -v "=1.10.6" --no-document
- bundle install --retry 5 --jobs=%NUMBER_OF_PROCESSORS%

This comment has been minimized.

@parkr

parkr Aug 22, 2016

Member

We should be able to use --without and specify the vendor dir via --path vendor. Is that invalid?

@parkr

parkr Aug 22, 2016

Member

We should be able to use --without and specify the vendor dir via --path vendor. Is that invalid?

This comment has been minimized.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I haven't tried the path option. Let me try that.

@XhmikosR

XhmikosR Aug 22, 2016

Contributor

I haven't tried the path option. Let me try that.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 22, 2016

Contributor

@parkr: path seems to do the job as far as I can tell:

Bundle complete! 40 Gemfile dependencies, 90 gems now installed.
Gems in the groups benchmark, site and development were not installed.
Bundled gems are installed into ./vendor/bundle.

Note that caching isn't actually active since the builds don't pass.

Contributor

XhmikosR commented Aug 22, 2016

@parkr: path seems to do the job as far as I can tell:

Bundle complete! 40 Gemfile dependencies, 90 gems now installed.
Gems in the groups benchmark, site and development were not installed.
Bundled gems are installed into ./vendor/bundle.

Note that caching isn't actually active since the builds don't pass.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 22, 2016

Contributor

On a side note @parkr, the site-default failure seems to be because Windows doesn't recognize exe/jekyll as ruby executable file.

Doing bundle exec ruby exe/jekyll new tmp/default-site should work. Should I try this in this PR?

Contributor

XhmikosR commented Aug 22, 2016

On a side note @parkr, the site-default failure seems to be because Windows doesn't recognize exe/jekyll as ruby executable file.

Doing bundle exec ruby exe/jekyll new tmp/default-site should work. Should I try this in this PR?

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Aug 23, 2016

Member

@XhmikosR, default-site script seems to run with simply exec instead of bundle exec
(I'm aware, its not entirely correct.)
Appveyor log showing passed default-site test.

Member

ashmaroli commented Aug 23, 2016

@XhmikosR, default-site script seems to run with simply exec instead of bundle exec
(I'm aware, its not entirely correct.)
Appveyor log showing passed default-site test.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 23, 2016

Contributor

I pushed my change to use ruby exe/jekyll in this PR and works, but it fails later https://ci.appveyor.com/project/jekyll/jekyll/build/349/job/x73k0syp9xmuvib3

Contributor

XhmikosR commented Aug 23, 2016

I pushed my change to use ruby exe/jekyll in this PR and works, but it fails later https://ci.appveyor.com/project/jekyll/jekyll/build/349/job/x73k0syp9xmuvib3

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Aug 23, 2016

Member

Ah yes.. I see it now.. double-drive-letter in the path..
bundle exec ruby is the better solution as it does not create a 'no-return-to-previous console' like exec does..
do you mind if I add ruby to my PR as well?

Member

ashmaroli commented Aug 23, 2016

Ah yes.. I see it now.. double-drive-letter in the path..
bundle exec ruby is the better solution as it does not create a 'no-return-to-previous console' like exec does..
do you mind if I add ruby to my PR as well?

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 23, 2016

Contributor

You can cherry pick it if you want. Or wait until this PR is merged and rebase then.

Contributor

XhmikosR commented Aug 23, 2016

You can cherry pick it if you want. Or wait until this PR is merged and rebase then.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Aug 23, 2016

Member

I'll push a commit manually referencing yours..
Thanks.

Member

ashmaroli commented Aug 23, 2016

I'll push a commit manually referencing yours..
Thanks.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Aug 23, 2016

Contributor

That is not the proper way to do it. Read about cherry picking or like I said, wait until this is merged and rebase.

Contributor

XhmikosR commented Aug 23, 2016

That is not the proper way to do it. Read about cherry picking or like I said, wait until this is merged and rebase.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Aug 23, 2016

Member

okay.. I'm aware of cherry-picking. I'll wait for this to get merged.

Member

ashmaroli commented Aug 23, 2016

okay.. I'm aware of cherry-picking. I'll wait for this to get merged.

XhmikosR added some commits Aug 13, 2016

Update AppVeyor config.
Install gems in ./vendor/bundle and cache those only.
script/default-site: add `ruby` in exec command.
Windows doesn't recognize `exe/jekyll` as Ruby executable so this should work everywhere.
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Aug 29, 2016

Member

@jekyllbot: merge +dev

Member

parkr commented Aug 29, 2016

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit bd4e746 into jekyll:master Aug 29, 2016

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
jekyll/lgtm Awaiting approval from at least 2 maintainers.
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Aug 29, 2016

@XhmikosR XhmikosR deleted the XhmikosR:appveyor branch Aug 29, 2016

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