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

Add dependecy constraints #58

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

shubhsboson
Copy link
Contributor

@shubhsboson shubhsboson commented May 12, 2023

Lock dependencies to known working versions to ensure builds work in future as well.

@shubhsboson shubhsboson changed the title Feature/add dependecy constraints Add dependecy constraints May 13, 2023
@shubhsboson
Copy link
Contributor Author

@captn3m0 Can you please look into this PR ?

Gemfile.lock Show resolved Hide resolved
Gemfile Show resolved Hide resolved
pages/cfp.md Outdated Show resolved Hide resolved
@shubhsboson
Copy link
Contributor Author

@captn3m0 All requested changes are done.

@shubhsboson
Copy link
Contributor Author

@captn3m0 Can you please review the PR again ? All requested changes are done.

Copy link
Member

@captn3m0 captn3m0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM. However, I'm unsure of how the deployment script works, what ruby/bundler version it currently runs on, and that might break things.

@kprovost
Copy link
Contributor

kprovost commented Jun 1, 2023

It runs rubygem-jekyll-4.3.2, ruby-3.1.4,1, ruby31-gems-3.4.10.

@shubhsboson
Copy link
Contributor Author

Following are the details from the container env, I made the changes on
Screenshot 2023-06-01 at 10 13 09 AM

@shubhsboson
Copy link
Contributor Author

@kprovost @captn3m0 , How do you suggest we proceed here ? And if we are all positive and ready to test please merge this to master.

@captn3m0
Copy link
Member

captn3m0 commented Jun 1, 2023

So the deployment is just a jekyll invocation for the deploy (jekyll build)? Without bundler, that seems to break (if you have a Gemfile present):

jekyll build
/usr/lib/ruby/3.0.0/bundler/definition.rb:524:in `materialize': Could not find jekyll-4.3.2, jekyll-sass-converter-2.2.0, addressable-2.8.4, colorator-1.1.0, em-websocket-0.5.3, i18n-1.12.0, jekyll-watch-2.2.1, kramdown-2.4.0, kramdown-parser-gfm-1.1.0, liquid-4.0.4, mercenary-0.4.0, pathutil-0.16.2, rouge-4.1.0, safe_yaml-1.0.5, terminal-table-3.0.2, webrick-1.8.1, sassc-2.4.0, public_suffix-5.0.1, eventmachine-1.2.7, http_parser.rb-0.8.0, concurrent-ruby-1.2.2, listen-3.8.0, rexml-3.2.5, forwardable-extended-2.6.0, unicode-display_width-2.4.2, ffi-1.15.5, rb-fsevent-0.11.2, rb-inotify-0.10.1 in locally installed gems (Bundler::GemNotFound)
	from /usr/lib/ruby/3.0.0/bundler/definition.rb:197:in `specs'
	from /usr/lib/ruby/3.0.0/bundler/definition.rb:254:in `specs_for'
	from /usr/lib/ruby/3.0.0/bundler/runtime.rb:18:in `setup'
	from /usr/lib/ruby/3.0.0/bundler.rb:171:in `setup'
	from /home/nemo/.gem/ruby/3.0.0/gems/jekyll-4.3.2/lib/jekyll/plugin_manager.rb:52:in `require_from_bundler'
	from /home/nemo/.gem/ruby/3.0.0/gems/jekyll-4.3.2/exe/jekyll:11:in `<top (required)>'
	from /home/nemo/.gem/ruby/3.0.0/bin/jekyll:25:in `load'
	from /home/nemo/.gem/ruby/3.0.0/bin/jekyll:25:in `<main>'

Running JEKYLL_NO_BUNDLER_REQUIRE=true jekyll build instead seems to fix it:

Configuration file: /home/nemo/projects/personal/hillhacks-website/_config.yml
            Source: /home/nemo/projects/personal/hillhacks-website
       Destination: /home/nemo/projects/personal/hillhacks-website/_site
 Incremental build: disabled. Enable with --incremental

But that means it will ignore everything in the Gemfile, so the benefits of this PR are ignored.

Using JEKYLL_NO_BUNDLER_REQUIRE still breaks, because it hits #57 (on my setup, freebsd is still pinning the sass-convertor to 2.x so it avoids the issue for now, but it might break in the future?).

Managed to fix #57 while debugging this, so that should give us a clean solution that keeps working. Will push my changes for #57 in this PR, and merge this PR.

We can switch our jekyll invocation in the deploy script to something like the following, after this is merged (Based on https://jekyllrb.com/tutorials/using-jekyll-with-bundler/):

git clone/pull [...]
bundle config set --local path 'vendor/bundle'
bundle install
bundle exec jekyll build

@kprovost
Copy link
Contributor

kprovost commented Jun 1, 2023

The deployment 'script' is a cron job that does /srv/www/Hillhacks && /usr/local/bin/git pull >/dev/null && /usr/local/bin/jekyll build >/dev/null.

It looks like Nemo has access to that box. The website lives in the wiki jail, and the job runs as www (sudo jexec wiki sudo -s -u www).

@captn3m0
Copy link
Member

captn3m0 commented Jun 7, 2023

Will get to this soon.

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

Successfully merging this pull request may close these issues.

3 participants