I contend that the current behavior of pre-deinstall-lua is a bug. The fix is changing this: if pkg.filecmp(sample_file, target_file) == 0 then os.remove(target_file) else pkg.print_msg("You may need to manually remove " .. target_file .. " if it is no longer needed.") end to this: pkg.print_msg("You may need to manually remove " .. target_file .. " if it is no longer needed.") My rationale for this is here: https://www.jetcafe.org/dave/pkgissue.html (I tried to use the correct component, but bugzilla will not let me do that for some reason.)
Thank you for the report and patch Dave. Can you include your proposed change as an attachment please. Apologies for the current inability to set the correct component. Note also that the URL provided is currently 404, I highly recommend attaching any supporting documentation to this issue, which ensures beyond being available, that relevent information does not stale out or bitrot over time, as tensds to be the case with external references
Created attachment 224781 [details] Proposed change to the sample Keyword's behavior
First of all, apologies about the 404, this is fixed (redirect issue). Next, jetcafe.org has been around since 1995 so I'm comfortable with leaving the rationale there. :) Finally, I've attached a patch as requested.
I'm not fully understanding the problem. Your website talks about nginx breaking because the configuration files are removed during the removal step of a `pkg upgrade`, but they should be recreated immediately when the package is upgraded/reinstalled. The only circumstance when nginx should break is when nginx is installed but the configuration files aren't. That seems to be what you're talking about, but I'm unclear how that is happening. Can you provide more detail?
(In reply to Adam Weinberger from comment #4) > they should be recreated immediately when the package is upgraded/reinstalled I have not observed this behavior consistently. Instead, with recent upgrades (1.14.6 -> 1.16.3) I've observed that in some cases the -dist files are not automatically installed (leaving no file in it's place), and in others they are. Note that this will likely be a separate (but related) issue, as I do not have enough actionable data to file a bug report yet (nor do I know if the inconsistent behavior is, in fact, a bug or something else). Still, this technically unrelated behavior is, in fact, the stimulus for my change request. Not installing -dist files is somewhat expected given that they are -dist files, but not expected by the person who wrote the nginx.conf file which includes them. I'm unclear why the current behavior of pre-deinstall-lua contains any notion of os.remove() ... I find this to be an unsafe practice. As far as I can tell, the mechanism in question is with the @sample keyword in the pkg-plist file. However, maybe this mechanism is insufficient or perhaps I am naive about my understanding of this idea? There are two cases I can see happening: 1) The port needs those files installed even if they are samples 2) The port does not need those files installed. The sample keyword appears to assume (2), not (1). Neither case implies deleting what is already there. Do you know of a way to increase verbosity of pkg in a way that sheds more light on this and related issues?
*.sample file will be registered with the hash. Editing it will cause pkg check -s to report the problem. * copied from *.sample will be used as the configuration file, but if the configuration file was not edited, you can delete it at the same time as uninstalling. That's what @sample is for, isn't it?
> if the configuration file was not edited, you can delete it at the same time as uninstalling. You are able to, yes. Whether it -should- be done is the subject of my bug report. We are observing the deletion of an unedited configuration file but not a subsequent replacement where replacement is necessary (e.g. the nginx port). I claim we don't need to delete this unedited file and that it's safer not to delete it in case issues like this arise. I do recognize (and should clarify if I haven't already) that there is a related issue with pkg deleting in-place configuration files but I don't yet have solid data so I can report it or even see if this is my issue and not FreeBSD's. This particular bug report is not intended to address the related issue.
OK, it sounds like the real issue here is that `pkg upgrade` is removing @sample files and not always replacing them. Removing the file isn’t the problem, so while this patch would work around it, it doesn’t address the underlying issue. We need to identify how and when the files are removed and not recreated. I haven’t seen this on any of my systems. I’m curious whether others are seeing this as well. Can you provide some more info about your environment? Are you building the packages yourself, or using FreeBSD-provided packages? How are you upgrading them? Does this happen on multiple machines, or just on one?
(In reply to Adam Weinberger from comment #8) > Removing the file isn’t the problem, so while this patch would work around it, > it doesn’t address the underlying issue. We need to identify how and when the > files are removed and not recreated. I agree that the underlying issue is not addressed. I would still support this patch being included as a policy idea. We have found that actually removing the -dist files prior to the upgrade consistently prevents the deletion of the @sample files, regardless of whether they have been edited or not. > Can you provide some more info about your environment? Yes. I build packages with poudriere: # poudriere version 3.3.2-10-g8e54ca5a for all my target sites. Each target site has different options directories and different package repositories. These streams are not crossed; upgrading a target site always uses the same options directories, package repositories, and build configuration. When I do these builds, I generally do them against the quarterly ports tree build I've picked at the time. Occasionally I cherry pick some ports from HEAD which have needed security upgrades. I use pkg(8) to manage package installations. I've been doing this for several years with no obvious issues. Each package upgrade goes like this: 1) Alter the repository configuration file to point to the new package repo 2) Upgrade pkg first with "pkg upgrade pkg" 3) Then let pkg have it's way with "pkg upgrade" This has always worked before and until this particular upgrade @sample files were not deleted. I did not notice the first instance of file deletion until I discovered that the file fastcgi_params was missing and nginx would not start. It was curious to me because I never edited that file (most of my nginx sites are templated and explicitly configured after we include this file). I did not discover this was pkg until -at a different site- the same thing happened. Let me know if you need more information.
The problem is during upgrade the predeinstall script from old version of the package is used at this time lua script has no way to be sure that the @sample it also present in the futur version or the package to be install. To do what you ask for, we would need to internalise this @sample into pkg directly, or to somehow extend the lua API in a way I don't know yet. (I would prefer extending the lua API :D)
(In reply to Baptiste Daroussin from comment #10) I struggle to think of a case where the pre-deinstall script needs to remove the active configuration, even if they are exactly the same. I am probably not understanding you completely here. :) Is there a good example?
When I remove a package I don't want any leftovers left behind by default. if pre-deinstall so pre-deinstall removes the files marked as sample if they are the same as the official @sample. Note it cannot be done in post-deinstall because the original .sample file will not be present anymore. if we want to do what you want to do we should move this to post-deinstall, then pkg needs to be modified to allow this (which is doable, takes more time and is probably desirable).
(In reply to Baptiste Daroussin from comment #12) > When I remove a package I don't want any leftovers left behind by default. Ah! This implied policy speaks directly to my issue. :) I think I understand the intent of this code, thank you for that. However, the condition where the package sample file is the same as it's installed configuration file is (at least for the nginx package) created by the person who installed the package. In the case of nginx, for example, the "mime.types-dist" file might be copied to "mime.types". That is a local policy decision, right? Maybe the user has written some other tool that expects that file to be there. Note well that even if the files are the same, the original nginx pkg install does not create a "mime.types" file by default: > pkg info -l nginx | grep mime.types /usr/local/etc/nginx/mime.types-dist > So to create "mime.types", the -dist version of the file might be manually copied by the pkg user, which I believe is a valid use case. Are there packages that actually -do- copy dist files to target files? If so, those would be counter examples. > if we want to do what you want to do we should move this to post-deinstall, then pkg needs to be modified to allow this (which is doable, takes more time and is probably desirable). I'm not very familiar with the distinction between pre-deinstall and post-deinstall, so I'm not immediately understanding why moving the same code to post-deinstall would somehow prevent the case where (using my example above) the mime.types file would not be removed on a pkg upgrade. Are you talking about using my patch in post-deinstall instead of pre-deinstall?
(In reply to dave from comment #13) In that case, mime.types-dist is installed in /usr/local/share/examples/nginx, and a symlink is created in /usr/local/etc to use it. It's my preference :)
(In reply to Tatsuki Makino from comment #14) Yes that's a valid use case. I don't think it should detract from another valid use case where you cp the dist file, perhaps intending to edit it later.
(In reply to dave from comment #15) As an additional note, nginx as configured will not work without a mime.types file, if you look at nginx.conf-dist. :)
somehow I lost track of this PR sorry about that! One thing we can propose is an option (I need to expand lua code to accept pkg option) to allow a user to decide they don't want the samples to be removed ever. The default remains the same, but for people using configuration management tools they can probably turn off the default "pre-deinstall" behaviour of @sample during via pkg.conf an option. Would this work for you?
(In reply to Baptiste Daroussin from comment #17) I lost track too. No worries. :) An option that we can set that persists across upgrades and doesn't require remembering that you need to add a switch for the behavior would work for me, yes. For some reason the original stimulus of this bug report is not happening anymore.