Skip to content
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

Install 32bit FTL on 32bit OS (even if 64bit architecture is detected) #2004

Merged
merged 2 commits into from
Jun 9, 2018

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Feb 24, 2018

By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes, and have included unit tests where possible.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)

Please make sure you Sign Off all commits. Pi-hole enforces the DCO.


What does this PR aim to accomplish?:

Try to determine if the user is running a 32bit OS on a 64bit system. If so, download the 32bit binary as we cannot expect the 64bit libraries to be present.

Should fix #1998 and many others + some issues we saw on Discourse.

How does this PR accomplish the above?:

Try to detect architecture dpkg uses to install packages. This will only work on Debian-based systems, but not on Fedora, etc.

Unfortunately, I cannot really test this at the moment as droplets with 32bit OS return

$ uname -m
i686

(which already leads to downloading the 32 bit library)


  • You must follow the template instructions. Failure to do so will result in your pull request being closed.
  • Please respect that Pi-hole is developed by volunteers, who can only reply in their spare time.

… If so, download the 32bit binary as we cannot expect the 64bit libraries to be present.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER requested a review from a team February 24, 2018 10:23
@DL6ER DL6ER changed the title Install 32bit FTL on 32 bit OS (even if 64 bit architecture is detected) Install 32bit FTL on 32bit OS (even if 64bit architecture is detected) Feb 24, 2018
@dschaper dschaper added PR: Code Review Required Open Pull Request, needs code reviewd PR: Approval Required Open Pull Request, needs approval labels Feb 24, 2018
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pihole-installer-usr-bin-pihole-ftl-no-such-file-or-directory/6006/34

@DL6ER DL6ER requested review from PromoFaux, dschaper, WaLLy3K, jacobsalmela and AzureMarker and removed request for a team March 8, 2018 12:31
PromoFaux
PromoFaux previously approved these changes Mar 8, 2018
Copy link
Member

@PromoFaux PromoFaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sound

local machine
machine=$(uname -m)

# This gives the architecture of packages dpkg installs (for example, "i386")
local dpkgarch
dpkgarch=$(dpkg --print-architecture 2> /dev/null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On fedora, is it ok that dpkg will not exist?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dpkg won't be there, but file will be:

file $(whereis file) | grep -v "64-bit" | grep "32-bit" &> /dev/null
if [ "$?" = 0 ]; then
  machine="i686"
fi

Which would technically work in both debian and fedora universes. But I'd only use it if 1] dpkg wasn't available, and 2] the architecture was identifying as x86_64 via uname -m.

AzureMarker
AzureMarker previously approved these changes Jun 9, 2018
Copy link
Contributor

@AzureMarker AzureMarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Fedora to see if the dpkg line would cause an error. No error was thrown, so it looks like this works.

@AzureMarker AzureMarker dismissed stale reviews from PromoFaux and themself via 4a75566 June 9, 2018 00:21
@AzureMarker AzureMarker merged commit c8bcd4a into development Jun 9, 2018
@AzureMarker AzureMarker deleted the tweak/32bitOS_on_64bitCPU branch June 9, 2018 00:56
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/new-install-on-raspberry-pi-os-on-esxi-virtual-machine-fails-to-install-correctly/41082/45

@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/new-install-on-raspberry-pi-os-on-esxi-virtual-machine-fails-to-install-correctly/41082/50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement PR: Approval Required Open Pull Request, needs approval PR: Code Review Required Open Pull Request, needs code reviewd Testing Required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants