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

PLIST fails to include all files #4222

Closed
th0ma7 opened this issue Oct 15, 2020 · 5 comments
Closed

PLIST fails to include all files #4222

th0ma7 opened this issue Oct 15, 2020 · 5 comments

Comments

@th0ma7
Copy link
Contributor

th0ma7 commented Oct 15, 2020

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 #4215 (comment)

  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
Copy link
Contributor

hgy59 commented Oct 16, 2020

@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
Copy link
Contributor Author

th0ma7 commented Oct 17, 2020

@th0ma7 can you describe the real error?

It all started with @ymartin59 comment #4211 (comment) 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
Copy link
Contributor

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
Copy link
Contributor Author

th0ma7 commented Oct 21, 2020

See cross-commenting in reference to #4221 (comment)
Complete list of packages needing a switch from BUILD_DEPENDS to DEPENDS available here #4221 (comment) (thnx @hgy59)

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

@hgy59 hgy59 mentioned this issue Oct 29, 2020
3 tasks
@ymartin59
Copy link
Contributor

So was not a framework bug

@hgy59 hgy59 removed the bug label Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants