-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Update Debian install documentation #12331
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
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1145ee6
Update Debian install documentation
MichaIng ba531fb
Merge branch 'master' into patch-1
MichaIng 4a9fbbd
Merge branch 'master' into patch-1
MichaIng cc818cb
Add linebreak
MichaIng 2081ad1
Merge branch 'master' into patch-1
MichaIng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
I'm wondering if we should keep this note; would this be a common situation? Perhaps we should keep this as an exercise for the "advanced user", as it would likely not be a standard situation.
Reason why I'm leaning towards removing it, is that the "notes" (targeting non-standard situations) now take up more space than the actual instructions for the "standard" case;
If we do think it's useful, perhaps we should move it inside each tab, and as a "non note", e.g.
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.
As stated above, IMO the "arch=" tag could be removed, leaving it up to DPkg to simply pull the native architecture of the installed OS/distro. Especially with
dpkg --print-architecturethe outcome should be exactly the same. If someone has some special setup, where parts of the system intentionally consists of different architecture binaries, one must have known about that "arch=" tag before and will know if/how to apply it. But using it, matches to what the installer does: https://github.com/docker/docker-install/blob/master/install.sh#L379Else I agree that the notes take too much space, so removing only the note about it should be fine as well. I added it mainly to be consistent with the note about the
lsb_releasecommand substitution note.An alternative would be collapsible notes, if you have the
pymdownx.detailsMarkdown extension enabled?Could be achieved with
<details><summary>HTML5 tags manually as well.But this doesn't "solve" the issue that the notes are much larger than the command block and adds the inconsistency that
dpkg --print-architectureis explained inside the tab, whilelsb_releaseis explained outside, even that for both basically the same is true: It is fine in 99% of cases and in the remaining rare cases, admin must have known already about their limitation on that system 😉.Uh oh!
There was an error while loading. Please reload this page.
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.
@thaJeztah
To push this a bit:
I finally suggest to remove
dpkg --print-architecturecompletely, the note and also the code.It is a no-op, as DPkg anyway installs packages for the major architecture (output of
dpkg --print-architecture), even if the version was lower. Only if a package is not available for the major architecture, it looks into foreign architecture lists. Declaring the architecture explicitly would break that automatic behaviour, even that it is most likely wanted: Hypothetically a new ARMv9 architecture may be created that is backwards-compatible to ARMv8=arm64 and the Docker repo does not yet ship binaries for it. If arm64 is added as foreign architecture, without declaring the architecture in the list file, DPkg would automatically install the arm64 package that runs on that system. If arm64 were incompatible with that hypothetical ARMv9 CPU, then the admin would not add arm64 as foreign architecture, so that APT/DPkg would error out with a meaningful error message.