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

Attemps to improve Debian builds... #1647

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Attemps to improve Debian builds... #1647

merged 3 commits into from
Jul 25, 2024

Conversation

farhi
Copy link
Contributor

@farhi farhi commented Jul 25, 2024

Some changes for the Debian builds. An attempt to see if it builds properly

@willend
Copy link
Contributor

willend commented Jul 25, 2024

Looks good and all tests check out OK, merging

@willend willend merged commit d4af19b into McStasMcXtrace:main Jul 25, 2024
16 checks passed
@farhi
Copy link
Contributor Author

farhi commented Jul 25, 2024

Hi Peter,
I'm afraid this PR is not yet fully functional. Some target binaries and libraries are missing/in the wrong place.
The basic tests do pass, but this is not enough for this PR.
I'll push a new PR soon...

@farhi
Copy link
Contributor Author

farhi commented Jul 25, 2024

Argh !
The postinst script has wrecked my system. There is a line (close to 216):

ln -sf ${MCCODE_BINDIR}/* .

which redefines links of executables in /usr/bin to themselves, because MCCODE_BINDIR=/usr/bin in Debian.
A test for such massive linking should be avoided...
I'll need to reinstall all tomorrow.

@willend
Copy link
Contributor

willend commented Jul 26, 2024

Indeed, I've been there myself...

Manually restoring bash and python links will get you most of the way, but yes likely a reinstall is needed...

I guess we should completely avoid the asterisk and only work on an explicit set of files?

@farhi
Copy link
Contributor Author

farhi commented Jul 26, 2024

I think the CMakeFiles are basically OK. What messed-up was the postinst/postrm. We should just get rid of these, and as you say, avoid any *.
Can you just confirm that the mkdist is now only used by the DEB/RPM build ?
What could be done is to first dereference these 2 scripts from elsewhere, see how we can still build, adapt, and in the end suppress them.

@willend
Copy link
Contributor

willend commented Jul 26, 2024

Hi E,

The mkdist stuff isused for the “legacy build scripts” of the buildscripts/ folder which is what was used historically for:

  • packages.mccode.org deb and rpms
  • “Monolithic” macOS bundles and Windows installers
  • Various other “local” install scripts, “src” tgz packages, documentation packaging

Both the current GitHub CI test scripts and conda packages should be unaffected as these use “standard CMake” (with mccode legacy path stuff set to off).

I would be in favor of simplifying the postinst/prerm to a bare minimum and better do too little than too much

@willend
Copy link
Contributor

willend commented Jul 26, 2024

(Long term we will ONLY use conda for installations on Windows and macOS - but not all features are ready on windows - dependencies for MCPL (and ncrystal for neutron side ))

@farhi
Copy link
Contributor Author

farhi commented Jul 26, 2024

Hi Peter !
OK, I have a stable local git to commit as a PR. Just tested in a vanilla VM installation, run/ display and removal are all OK.
I only touched the deb scripts, and added a test to avoid affecting directly the /usr/bin in non-legacy location 9e.g. Debian standard).

Regarding the postinst/postrm, I think we can get rid of the 'launcher' section by the end, as the 'launcher' directory does not exist afaik.

desktop=$(${MCCODE_RESOURCEDIR}/launchers/*.desktop 2> /dev/null | wc -l)
if [ -d ${APPTARGET} -a -d ${MCCODE_RESOURCEDIR}/launchers -a "$desktop" != "0" ];
then
    cd ${APPTARGET}
    ln -sf ${MCCODE_RESOURCEDIR}/launchers/*.desktop .
else
    echo "No APPTARGET folder \"${APPTARGET}\" or ${MCCODE_RESOURCEDIR}/launchers/.desktop: dropping link creation"
fi
logo=$(${MCCODE_RESOURCEDIR}/launchers/*.png 2> /dev/null | wc -l)
if [ -d ${LOGOTARGET} -a -d ${MCCODE_RESOURCEDIR}/launchers -a "$logo" != "0" ];
then
    cd ${LOGOTARGET}
    ln -sf ${MCCODE_RESOURCEDIR}/launchers/*.png .
else
    echo "No LOGOTARGET folder \"${LOGOTARGET}\" or ${MCCODE_RESOURCEDIR}/launchers/.png: dropping link creation"
fi

will commit locally, then PR.
Much better than yesterday for sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants