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

Set *_temp_path location to $ServRoot/ #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jirutka
Copy link

@jirutka jirutka commented Apr 27, 2017

Linux distributions build nginx with options like --http-client-body-temp-path= to set default location of directories for temporary files under /var/tmp/nginx or /var/lib/nginx.
The problem is, when running tests as part of the package build process, these directories are not writtable. Therefore it's needed to override location of these directories in the generated nginx.conf.

proxy_temp_path "$ServRoot/proxy_temp";
fastcgi_temp_path "$ServRoot/fastcgi_temp";
scgi_temp_path "$ServRoot/scgi_temp";
uwsgi_temp_path "$ServRoot/uwsgi_temp";
Copy link
Member

Choose a reason for hiding this comment

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

Shall we put these to the end of the http {} block? Your additions here will change the nginx.conf line numbers of the user configuration snippets, and thus will break existing tests checking line numbers of nginx.conf in some error messages.

Copy link
Member

Choose a reason for hiding this comment

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

Another thing is that you are assuming ngx_proxy, ngx_fastcgi, ngx_scgi, and ngx_uwsgi modules are always enabled in the nginx used in testing, which is apparently not the case. Sometimes we explicitly want to test a 3rd-party module when some or all of these standard nginx modules are absent.

Copy link
Author

Choose a reason for hiding this comment

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

Your additions here will change the nginx.conf line numbers of the user configuration snippets, and thus will break existing tests checking line numbers of nginx.conf in some error messages.

I didn’t know that, I’ve moved them to the end of http block.

Another thing is that you are assuming ngx_proxy, ngx_fastcgi, ngx_scgi, and ngx_uwsgi modules are always enabled in the nginx used in testing, which is apparently not the case.

I know, but I don’t know how to enable them conditionally only when needed. Any idea?

Copy link
Member

Choose a reason for hiding this comment

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

@jirutka I suggest

  1. you use your own build of nginx or builds that are not doing clever things with those temp directory paths, or
  2. Use --- main_config to specify those settings yourself in your test cases.

Linux distributions build nginx with options like
`--http-client-body-temp-path=` to set default location of directories
for temporary files under /var/tmp/nginx or /var/lib/nginx.
The problem is, when running tests as part of the package build
process, these directories are not writtable. Therefore it's needed to
override location of these directories in the generated nginx.conf.

Upstream-Issue: openresty#63
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.

2 participants