search

SynoCommunity / spksrc

Cross compilation framework to create native packages for the Synology's NAS
https://synocommunity.com
Other
3.02k stars 1.23k forks source link

PLIST fails to include all files #4222

Closed th0ma7 closed 3 years ago

th0ma7 commented 3 years ago

Current PLIST code does not include each and every files it should. In some cases there are dependencies left aside. Problem is really obvious with fossil-scm and seems to have been existing since 2019+.

Setup

Package Name: fossil-scm Package Version: any Linked issues: #4221, #4211, #4215

NAS Model: all NAS Architecture: all DSM version: 6.1, 6.2.3

Proposal

Option 1) Rewrite the PLIST code: Interestingly this option refers to a previous comment at https://github.com/SynoCommunity/spksrc/issues/4215#issuecomment-707119279

  1. Review the spksrc.install.mk in order to rename the post-processed file $(WORK_DIR)/$(PKG_NAME).plist as something else (e.g. ? $(WORK_DIR)/$(PKG_NAME).files) to free-up the .plist extension
  2. Create a new spksrc.plist.mk using similar COOKIE handling in order to create an $(WORK_DIR)/$(PKG_NAME).plist for each packages at post-install time. Include in this .mk the ability to check (spksrc.plist-check.mk?) that every file is really existing so if an error happens it is caught at the dependency level right away instead of at the complete end of the build process. This new mk would be called after spksrc.install.mk in probably (to be confirmed) spksrc.cross-cc.mk, spksrc.cross-go.mk and spksrc.install-resources.mk
  3. Re-write and rename the existing plist code (currently spksrc.plist.mk) running at the end of spk build process in order to simply PLIST_TRANSFORM over all $(WORK_DIR)/$(PKG_NAME).plist files available from within the $(WORK_DIR).
hgy59 commented 3 years ago

@th0ma7 can you describe the real error? I never ever had a problem with this (but I used only the synocommunitity/spksrc docker image). Just verified with current master and fossil-scm. It successfully strips bin/fossil and the folder install/var/packages/fossil-scm/target/bin contains fossil, openssl and c_rehash.

th0ma7 commented 3 years ago

@th0ma7 can you describe the real error?

It all started with @ymartin59 comment https://github.com/SynoCommunity/spksrc/issues/4211#issuecomment-708379978 where openssl libraries are not being included in fossil-scm package.

From there the idea was that the issue might come from the recent changes I did in the framework thus I've tried to find since when fossil-scm issue was undergoing (manual git bisecting). At a certain point I started having doubts as

  1. previous package of fossil-scm doesn't include openssl libraries neither and
  2. behavior didn't change at all while moving back in time which made no sense that this issue might have been undergoing since early 2019++ without noticing it before.

Just verified with current master and fossil-scm. It successfully strips bin/fossil and the folder install/var/packages/fossil-scm/target/bin contains fossil, openssl and c_rehash.

Voilà! You are absolutely right! Files do get properly installed in target/lib and target/bin but as theses are BUILD_DEPENDS and not DEPENDS they do not get packaged! Thus behavior is to be expected! Thus there is "no" problem there.

Now, making some mileage on @ymartin59 comment, still, should BUILD_DEPENDS libraries be included or not? Well, if it is statically linked probably not but most packages are using shared libraries all the way. Thus perhaps the library only portion may need to be packaged after all for BUILD_DEPENDS ?

ymartin59 commented 3 years ago

In fact BUILD_DEPENDS components are not expected to be included in package. Its typical usage is to generate static libraries (.a file) which is embedded at static linking when creating executable binary.

Here, I guess something has changed in fossil build chain which now use dynamic linking instead of static linking (may a change in configure default options... And so it requires now to declare "openssl" as DEPENDS, not BUILD_DEPENDS. And it may be just "enough".

th0ma7 commented 3 years ago

See cross-commenting in reference to https://github.com/SynoCommunity/spksrc/pull/4221#issuecomment-713894220 Complete list of packages needing a switch from BUILD_DEPENDS to DEPENDS available here https://github.com/SynoCommunity/spksrc/pull/4221#issuecomment-711067242 (thnx @hgy59)

Another approach (perhaps longer-term) could be to package only the lib portion of BUILD_DEPENDS...

ymartin59 commented 3 years ago

So was not a framework bug