Code review comment for lp:~gary-wzl77/squid/snap_package

>> should be done without adding a single monolithic set of options tied to the snap environment

> That's impossible

I obviously think it is possible. AFAICT, your "Because the ultimate goal is to publish squid snap into the stable channel" explanation does not prove that the only way to achieve this ultimate goal is to have a "monolithic set of options tied to the snap environment".

> Snap that can only read and write in its own namespace is recommended and enforced, if we wanna publish it into stable channel.

I am not against teaching Squid to obey some kind of a "namespace". We already have "./configure --prefix" and "squid -n". It is possible that more knobs like that are needed (but it is on you to prove that the existing knobs are insufficient or too awkward to use in a snap context -- I do not see that proof in your comments but please point me to it if I missed it).

To better understand why I do not like your implementation, take a step back and assume that there is not just one snap-like environment, but ten. It does not matter what they are called. You can call them Snap v1, Snap v2, ... or Docker, Snap, Chroot, ... or something else. Do we want to add --enable-snap-v1, --enable-snap-v2, --enable-snap-v3, ... options and then deal with all their weird combinations in the code, while being unable to test most of them? No. Does that "no" mean that we do not want to support Snap v1, Snap v2, ...? No! It only means that you need to (a) use existing configuration knobs and (b) generalize the missing knobs that you want to add.

For example, if --prefix and/or -n are enough to support snap "install path", use those existing configuration features. If they are not enough, describe what is missing in generalized terms and propose adding knobs for that generally useful support (using snap as an example).

Similarly, do you need a custom prefix for shared memory segment names? Does Squid already support a customer prefix for those names? If yes/no, then it would be OK to propose a ./configure or runtime option that adds such support, but that option is not going to be --enable-snap, it would be something like --ipc-prefix.

Hope this clarifies. To avoid doing a lot of work that is going to be rejected at the end, I recommend getting a preliminary Project approval in advance, based on a brief description of what Squid changes you want to make and _why_ they are necessary.

N.B. As for licensing conflicts, I do not yet understand why your proposal would create any new ones, but I will leave it for you and Amos to battle that out.