From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 78DAB158170 for ; Fri, 19 Jul 2024 09:20:21 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id AFEBEE2A0E; Fri, 19 Jul 2024 09:20:16 +0000 (UTC) Received: from mail-lf1-x134.google.com (mail-lf1-x134.google.com [IPv6:2a00:1450:4864:20::134]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 68DB7E2A0B for ; Fri, 19 Jul 2024 09:20:16 +0000 (UTC) Received: by mail-lf1-x134.google.com with SMTP id 2adb3069b0e04-52e99060b0dso1473464e87.1 for ; Fri, 19 Jul 2024 02:20:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1721380815; x=1721985615; darn=lists.gentoo.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=8Y4QiEFwDLSm8o+JjRXONtrWMq+hqD9rK+/gBkMclJU=; b=Hn4k7bIAnfnfOGt9BTksXa/uFgkjJB2KLhzYKWYjTQ2zLKeei3ULN6tMzETmeuv7hJ cR4+axtCRISrZJnUIl/x97y3gzNgQT4708QWWxkzh3vC7Opk4YV8R8Rqcz4yWtvjqq3X 8jKBxH17arlz0SCBB+WWh+A8KsQULGNOdpDNNgmGki/RI7UK/Lz/nWr8zh+WionfyuNC CEinoPXoRLvvBO0mC25/LDNJMkrJP8DYXwyo+tfMn6bI6rzdq0gpNdk5PDAWK8fGOaxH aPqPijysNohYTciN5j4AJjXSNVsLcGSR9gWUlN/sNbGCasCi8zsmJtTBblVEwrMWyXaO c6Lw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721380815; x=1721985615; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=8Y4QiEFwDLSm8o+JjRXONtrWMq+hqD9rK+/gBkMclJU=; b=epTHHKuAqBGmBDebWVecJqFnoMEsLeiB12cnBnWzIAbxvPlsqbXrbJRhNPm8jTDmpL N22eQtZryawddgVNHRKyqMax5eb4Mn+lkPQA6sqPpIZJG5Q/c5CrRW0Wot6Xo9IPSV09 bQHTW2zbWGfZlebEAYMuPY9tj7OBp9FRDAHgpQ/WSFOdcKznjhPSGwSEZf+h51yCMMim jjKiBDxkLGvn1nPpUZT7cCcnstSAQzctmp3agbUQazWXnFROi4aiuNknxK8xesbpnJ1s 70kEhgtdbPSs28sQUK9qo8C75dzF1fajz8jEQ6gFnC4nssYUGM4GUvO9BKuyinbKasXF XJhw== X-Gm-Message-State: AOJu0YyQ0XSPcqnoJY4BOfhDJOtJOE4lqmOXDa3bnTNENpDsfTgGMM62 YfTjs4IP+yIivOIP/d7obeatiJ5YwfrcT8h8GpkNHma9U8+9ojetV2vg4A== X-Google-Smtp-Source: AGHT+IFQ5tUTlsu/tw3959NDM8mM29Q9ylKqjyTobKm6JBhMNrGDg+cp359fPm7LdBj30PKkoeEGpw== X-Received: by 2002:a05:6512:239c:b0:52c:dd7d:3fd4 with SMTP id 2adb3069b0e04-52ee53b4c56mr5654030e87.25.1721380814290; Fri, 19 Jul 2024 02:20:14 -0700 (PDT) Received: from gentoo-desktop ([62.244.50.57]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a7a3c922c8fsm1870866b.145.2024.07.19.02.20.13 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Jul 2024 02:20:13 -0700 (PDT) Date: Fri, 19 Jul 2024 09:20:08 +0000 From: Zurab Kvachadze To: gentoo-dev@lists.gentoo.org Subject: Re: [gentoo-dev] [RFC PATCH 04/19] www-servers/nginx: add nginx-r5.initd Message-ID: <347alr7mlwdxkyxfx6fxyki3ywb2cae2otzb74yd374ds4jelr@a425qznt4gta> References: <20240717120553.31866-1-zurabid2016@gmail.com> <20240717120553.31866-5-zurabid2016@gmail.com> <16a8315d4c23c3d6bd549c431a2362253a9b860e.camel@gentoo.org> Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <16a8315d4c23c3d6bd549c431a2362253a9b860e.camel@gentoo.org> X-Archives-Salt: e54b52f0-c7ec-4bac-9e28-3f6e380ef055 X-Archives-Hash: 8f58f0b07eeb6fd1df1557d4370f5699 I apologise for such delayed response. We're having major electricity outages here and I couldn't find the possibility to address your review. Nonetheless, here we are :) On Wed, 2024-07-17 at 08:41 +0000, Michael Orlitzky wrote: > On Wed, 2024-07-17 at 15:05 +0300, Zurab Kvachadze wrote: > > > > +NGINX_CONFIGFILE=${NGINX_CONFIGFILE:-/etc/nginx/nginx.conf} > > +pidfile=${NGINX_PIDFILE:-/run/nginx.pid} > > [...] > > In fact, I would delete the NGINX_PIDFILE variable entirely, leaving it > at /run/${RC_SVCNAME}.pid. There's no reason for anyone to change it. > You can force the daemon to use that path with -g "pid ${pidfile}", > relieving you of the responsibility to coordinate with the config file. I haven't even known of this OpenRC feature! As you explained it, the removal of NGINX_{CONFIG,PID}FILE totally makes sense. Should I also add '-g "pid ${pidfile};"' to command_args for your example with 'nginx-internal' to work automatically without any conf.d tinkering? Anyway, I will apply both (or only the first one) changes to the v2 of patch series. > > +depend() { > > + need net > > + use dns logger netmount > > +} > > I don't think "need net" is right here because nginx starts fine on > localhost or 0.0.0.0 until you configure it to use a specific address. Initially, I've also wanted to remove the aforementioned line, but my end decision was to leave it alone, because I wasn't really sure of its uselessness. Now that you've explained it (and after reading the service writing guide) I see that this directive is of no use and, thus, will be removed in the second version of this MR. > > +start_pre() { > > + checkpath -d -o root:root /var/tmp/nginx || return 1 > > +} > > + > > The old script had, > > if [ "${RC_CMD}" != "restart" ]; then > configtest || return 1 > fi > > here. If "nginx -t" produces better error messages that plain nginx, > that might be the reason. But otherwise it's redundant. With nginx.eclass, the build flags now configure NGINX to store its temporary files in /var/tmp/nginx. Yet, NGINX doesn't create it by itself, but rather aborts its execution if /var/tmp/nginx doesn't exist. This is why I added the 'checkpath' line. As for the configtest, it's redundant. When NGINX is executed, it checks the configuration, making it unnecessary to check the same thing beforehand (the error reporting is identical anyway). configtest just duplicates NGINX's work and this may lead to twice the time to bring NGINX up, which may matter if large "geo module bases" are used[1]. [1]: https://bugs.gentoo.org/481456#c0