Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CI: Add FreeBSD build tests for staged commits #1948
CI: Add FreeBSD build tests for staged commits #1948
Changes from 26 commits
7cd4b0a45b311f626fa2e8a1356d7285e223a432fe32c0a666979b08bd56e8125f2cf5cc23c00f3f72486ddd23cf6fbcaf3bc138e8f42e001dcc91dd1cf8904021851317566f8a8d8e2ee91e3330023e05ec8fe84593fd9e3ab17cc81425893174c7File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that ccache optimization is removed, why do we need these customizations? Please either remove them (if they are not needed) or add a YAML comment to explain them (otherwise).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without these, in practice, we would still use clang, but it would be called 'gcc'.
Would that be confusing to you or anyone else looking at it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. If compiler naming is the only effect of these customizations, then please remove these customizations.
I am sure that will be confusing or at least puzzling to some folks (possibly including me). However, customizing this confuses or puzzles some folks as well (obviously including me)! We are selecting among two evils here.
This PR is not about improving FreeBSD (or autoconf or whatever) approach to naming things. With all other factors being equal, the closer we stay to FreeBSD customs/conventions when testing on FreeBSD, the better, regardless of how problematic those customs/conventions are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have no idea about what FreeBSD customs/conventions are. I suspect that just like MacOS does, they install portability shims to make nonportable software content.
I don't agree we should follow problematic customs or conventions, especially when it comes to explicitly naming what compiler we target. But this is not a hill I'm going to die for
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this context, we do: Whatever happens by default (i.e. without our customizations) is likely to be customary/conventional.
You may be right, but it is their business. Folks using FreeBSD ought to be accustomed to whatever FreeBSD is doing.
With all other factors being equal, I think we should approximate, to the extent it is feasible/practical, what a typical admin sees/does when compiling Squid on FreeBSD. Any unnecessary customizations are unwanted in this "Does Squid build on FreeBSD?" context. We are not trying to build on FreeBSD configured "our way". We are trying to build on FreeBSD configured "their way" (again, to the extent it is feasible/practical).
IMO, this design decision can and should be made based on merits rather than personal sacrifices. And since many other decisions are based on the same basic "what are we testing?" question, I hope that this discussion helps reduce the time spent on re-arguing them in the future. Every time we are about to add a customization to a build environment, we have to pause and ask ourselves: Is it justified? Do our actions really have to differ from what admins are going to do? And, if yes, where do we explain the necessity of this customization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't FreeBSD defaults, these are autotools' defaults. If we were to base our project on - say - cmake or bzl, we might be seeing something different. If FreeBSD has any default, it's
ccandc++, but autoconf is opinionatedAs shown above, we aren't. I can agree to use CC=cc and CXX=c++, if that helps
This is one of the decisions where it's hard or even impossible to agree based on merits, because we are comparing outcomes that boil down to different axes. On one side there's "let's be explicit about what we are testing against", and on the other there is "let's add as little context as possible". They are mutually exclusive, and achieve different goals. These goals cannot be ranked objectively, only subjectively; we are finding it hard to disagree because I think one goal is more valuable, and you think another one is. Our processes grant more power to the reviewer than to the author of a PR, therefore my statement, which could be expanded to "I disagree with your assessment, but I rank the goal of landing this PR more than I rank the goal of not landing it because I fixate on what I consider a valuable but minor detail, therefore I'll follow your opinions although I disagree with them".
So, let's have it your way and move on :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... which may depend on the OS. At any rate, my arguments apply to autotools' defaults just as well.
It does not help enough: The alleged problem of the customization itself being unnecessary and uncustomary would remain regardless of the chosen custom compiler name/alias.
Hm... I did not make "be explicit about what we are testing against" and "let's add as little context as possible" arguments (or their opposites). Bringing these arguments into this discussion appears to miss my primary argument/point -- avoiding customizations.
I disagree that adding
CC=clangis "let's be explicit about what we are testing". Instead, it is "let's test clang build". Explicitness of that decision expression is secondary to the decision itself. If the wrong decision can be avoided in the first place, these secondary details do not matter.While I agree that it is "hard to agree based on merits", I still think it is worth reaching such an agreement. As for "comparing outcomes [using] different axes", a merit-based argument should consider multiple angles if multiple angles are relevant to the decision, of course.
Those two goals are not the goals I am comparing because rejection of significant build environment customizations can and should be done before these secondary goals are considered.
That statement is misleading IMO. Both roles have multiple powers. Those power sets are different, but neither set is strictly "more" or "less" than the other.
Unfortunately, the current approach results in an enormous waste of our time as we keep revisiting (and spending a lot of time on) the same basic decisions again and again and again. "[Eventually] moving on" is better than not moving, of course, but I was hoping for more efficient movement (through shared agreement regarding such basic principles as avoiding build environment customizations)...