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

Wrong PATH for downloaded ruby for heroku-22 and above? #127

Closed
tkrevh opened this issue Aug 19, 2024 · 12 comments
Closed

Wrong PATH for downloaded ruby for heroku-22 and above? #127

tkrevh opened this issue Aug 19, 2024 · 12 comments
Assignees

Comments

@tkrevh
Copy link

tkrevh commented Aug 19, 2024

Hello!

I'm trying to upgrade to heroku-22 and I'm running into issue that erb command needed by start-nginx script is not found.

Could it be, that the following line:

echo "export PATH=\"\${HOME}/${vendored_ruby_dir}/bin:\${PATH}\"" > "${BUILD_DIR}/.profile.d/ruby.sh"

should actually reference ${BUILD_DIR} instead of ${HOME} ?

Like this:
echo "export PATH=\"\${BUILD_DIR}/${vendored_ruby_dir}/bin:\${PATH}\"" > "${BUILD_DIR}/.profile.d/ruby.sh"

If you look at line 40, the folder is created like this:
mkdir -p "${BUILD_DIR}/${vendored_ruby_dir}"

But when setting the path, another variable is used.

This works fine on heroku-20 and earlier versions as it seems they came with preinstalled Ruby.

@dzuelke dzuelke self-assigned this Aug 22, 2024
@dzuelke
Copy link
Contributor

dzuelke commented Aug 22, 2024

No, ${HOME} is correct - that .profile.d script is evaluated at dyno startup.

I can't reproduce this - Ruby gets installed, and the path set correctly, for my simple test case.

Are you using this in combination with another buildpack by any chance?

@dzuelke
Copy link
Contributor

dzuelke commented Aug 22, 2024

@tkrevh please provide output from the build, as well as logs from app startup. If you're using a custom Nginx config, please share that if you can. Also, a heroku run bash where you do echo $PATH and which erb would be helpful. Plus, like I already mentioned, any info on additional buildpacks you're using.

@chap
Copy link

chap commented Aug 23, 2024

heroku-22+ no longer includes ruby

The stack no longer includes a system Ruby installation. This will not affect the vast majority of users, since Ruby apps will use the Ruby installation provided by the Ruby buildpack.

@dzuelke
Copy link
Contributor

dzuelke commented Aug 23, 2024

That's not the issue though, @chap - the buildpack installs Ruby for this exact reason. Must be something else.

@tkrevh
Copy link
Author

tkrevh commented Aug 27, 2024

@dzuelke yes, we're using a couple of other buildpacks

Our custom nginx config is here:

daemon off;
#Heroku dynos have at least 4 cores.
worker_processes <%= ENV['NGINX_WORKERS'] || 4 %>;

events {
        use epoll;
        accept_mutex on;
        worker_connections 1024;
}

http {

    # gzip support (don't gzip in the application, nginx will do it as good and likely better, and keeps it simplified)
    gzip on;
    gzip_http_version 1.0;
    gzip_proxied any;

    # values 4-6 seem to be the sweet spot between CPU and compression, but we'll default to 2 as we're not paying
    # for bandwidth on heroku, so this should use the fewest CPU cycles which are more scarce.
    gzip_comp_level 2;
    gzip_min_length 512;
    gzip_disable "MSIE [1-6]\.";
    gzip_types text/plain
             text/html
             text/xml
             text/css
             text/comma-separated-values
             text/javascript
             application/x-javascript
             application/atom+xml
             application/json;


    server_tokens off;

    log_format l2met 'measure#nginx.service=$request_time request_id=$http_x_request_id';
    access_log logs/nginx/access.log l2met;
    error_log logs/nginx/error.log;

    include mime.types;
    default_type application/octet-stream;
    sendfile on;

    #Must read the body in 5 seconds.
    client_body_timeout 5;

    upstream app_server {
            server unix:/tmp/nginx.socket fail_timeout=0;
    }

    server {
            listen <%= ENV["PORT"] %>;
            server_name _;
            keepalive_timeout 5;
            client_max_body_size 0;

            location / {
                    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
                    proxy_set_header Host $http_host;
                    proxy_redirect off;
                    proxy_pass http://app_server;
            }


            location /staticfiles/ {
                root /app/app;
            }
    }
}

I will provide other requested information in a couple of days. We're preparing a major release and I've put this aside for a few days.

@dzuelke
Copy link
Contributor

dzuelke commented Aug 29, 2024

That would be very useful, @tkrevh. We're still unable to reproduce this.

@dzuelke
Copy link
Contributor

dzuelke commented Sep 3, 2024

Any news here, @tkrevh? Can you reproduce this easily on a new app?

@tkrevh
Copy link
Author

tkrevh commented Sep 3, 2024

I'm sorry, we just had a major release and our focus is elsewhere. I will revisit this later in the week.
Sorry and thank you for your patience!

@dzuelke
Copy link
Contributor

dzuelke commented Sep 10, 2024

All good, I know how it is sometimes, @tkrevh :)

@tkrevh
Copy link
Author

tkrevh commented Sep 12, 2024

@dzuelke thank you for your patience. I got back to this... just to realize what a stupid mistake I made.

I overlooked the fact, that we were using a fork of this buildpack.
I've merged the new compile script and added the missing start-nginx-static script to our forked buildpack repository,
and it's working.

Totally my fault, so sorry!

The only reason we're keeping our own fork of the buildpack is the correct support for SIGTERM.
There is an open PR for this (and we've been using this fix in production for over 3 years so it's proven):
#88

Any chance we could get it merged?

@dzuelke
Copy link
Contributor

dzuelke commented Sep 12, 2024

Ah, that explains a lot! :)

We'll take a look at #88 - the PHP buildpack does something similar for its Nginx build (and HTTPD, and PHP-FPM).

@dzuelke dzuelke closed this as completed Sep 12, 2024
@tkrevh
Copy link
Author

tkrevh commented Sep 12, 2024

Thank you, appreciate it 🙏

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

No branches or pull requests

3 participants