search

LMS-Community / slimserver-vendor

Third-party software used with Lyrion Music Server
https://lyrion.org
42 stars 68 forks source link

Feature/x86 rename knob #38

Closed fsbruva closed 6 years ago

fsbruva commented 6 years ago

This PR encompasses 4 commits.

  1. The code was refactored to use PERL_BIN, rather than ARCHPERL. This required defining ARCH earlier, rather than after the build completed. It also required inverting the logic for the OS test, and moving the PERL_CC tests later.
  2. An unreported bug was fixed: the dependency tests checked for cpp, instead of $GPP.
  3. Code was refactored to move all dependency checks to the same location within the script.
  4. Added new script knob to allow users with certain architectures (i686-linux, for example) to prevent the build script from renaming to i386-linux.

Closes #2.

mherger commented 6 years ago

What problem does this change solve?

fsbruva commented 6 years ago
  1. The PERL_CC test currently uses ARCHPERL, and thus assumes that ARCHPERL is the same as PERL_BIN. This is a logic error, because PERL_BIN (the version of Perl being used by the build) may differ from ARCHPERL, giving erroneous feedback to the user. I see this as a bug.
  2. All C/C++ programs in buildme.sh set CPP=$GPP, but only cpp is checked for existence. This is a regression, and I see it as a bug.
  3. Commit a7d7e39 closes a four year old unresolved issue reported by a user. I thought it was left open because it was still valid, and needed work. If you tell me the reported issue isn't actually a problem, I will rebase and skip this commit.

If you want, I can do a rebase, and skip cebee17 - my code layout preferences are not the same as "solving problems."

fsbruva commented 6 years ago

Will separate the first two bugfixes into their own PR's.

@mherger Do you see #2 as a valid issue, or one that ought to be closed without resolution?