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.

Sign up for GitHub

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

Jump to bottom

Add symlink creation to framework #3852

Merged
merged 7 commits into from
Jan 23, 2020
Merged

Add symlink creation to framework #3852

merged 7 commits into from
Jan 23, 2020

Conversation

hgy59
Copy link
Contributor

@hgy59 hgy59 commented Jan 12, 2020

Motivation: This PR was initiated by a question by @ymartin59
Linked issues: #3780 (comment)

There is no automatic symlink creation as assumed in the related comment.
It turned out that there are always only some particular binaries to add to the path.

This solution handles two new Makefile variables:

SPK_COMMANDS is used for simple links from ${SYNOPKG_PKGDEST} to /usr/local. The parent folder (bin or sbin) must be provided since the solution is implemented independent.

SPK_LINKS is provided for links with different source and targets. This supports a different target or to rename the command. The links are defined as <full_target>:<pkg_relative_source>

Examples:
SPK_COMMANDS = bin/git bin/git-cvsserver bin/git-receive-pack bin/git-shell bin/git-upload-archive bin/git-upload-pack
SPK_LINKS = /usr/local/bin/tmux:bin/tmux-utf8

Checklist

  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully

Remarks

  • most of the related packages are not updated now (most of the cli tools don't have data to migrate anyway)
  • synocli-* have updated SPK_REV

Related packages

Updated and tested packages are all of /diyspk plus:

  • beets
  • borgbackup
  • chromaprint
  • comskip
  • git
  • he853
  • itools
  • lua
  • mediainfo
  • rsnapshot
  • synocli-disk
  • synocli-file
  • synocli-net
  • tcl
  • tmux
  • zsh
  • zsh-static

Packages to update

  • synocli-file due rmlint fix and mc links
  • synocli-disk (description for archs not supporting DAR fixed)
  • synocli-net (optional, description for ppc853x fixed)

hgy59 added 5 commits January 12, 2020 14:30
@hgy59
PLIST cleanup: use documented types only (i.e. bin,lib,lnk,rsc)
73f5155 
- replace types etc,shr,etc. by rsc
- avoid types bin,lib for non strippable files
@hgy59
cross fixes
0b892ed 
- rmlint: install as executable
- ddrescue: add compiler flags removed by patch (add optimization)
- screen: remove chown from install, install without version postfix
@hgy59
Enhance framework for symlinks
de3060d 
- add generation and removement of symlinks defined in Makefile
- create folders for links on demand
- make call_func of installer silent
- fix error logs for unexisting INST_VARIABLES folder
@hgy59
update command line tools
19fe58b 
- define symlinks in Makefiles
- remove service-setup.sh scripts used for link creation only
- remove BETA from diyspk packages
- use variable for optional description in synocli-disk and synocli-net
- update tmux icon
- make package icons square (detox, rmlint, testdisk)
- use unique gnu icon (less, ddrescue)
- ddrescue: remove pre DSM 6 install scripts (dsm-control.sh, installer.sh)
@hgy59
Update changelog of synocli-file
c4e22a9
@hgy59 hgy59 requested a review from ymartin59 January 12, 2020 14:52
@ymartin59
Copy link
Contributor

@hgy59 Marvelous. I had the very same idea. I wonder why this PR contains changed 146 files. I will review.

@chickenandpork
Copy link
Member

:) awesome. Thank you!

@chickenandpork
Copy link
Member

If it’s too many changed files — perhaps clouding one-change-one-PR — make the change with one package edited to use, then PR the rest of the changes later?

@hgy59
Copy link
Contributor Author

hgy59 commented Jan 15, 2020

I cleand up all the PLIST files (that is a lot)
Regarding the symlink creation I propose to rework all in one, even most of the packages are not updated now. Since this is a framework change, it is better to remove all the old versions as any package could be used as a template for new packages.
Further more it is difficult for contributers to know/remember all the changed concepts and incorporate such while updating existing or creating new packages.
(e.g. there are still packages that are not updated to the DSM6 service and user concept - and we are expecting DSM7 this year!).

There are still CLI packages that do not create symlinks yet (or do not compile, or I left for later).
These are for future PRs:

  • mono
  • openssh
  • rsnapshot
  • gnupg
  • htop
  • imagem0agick
  • irssi
  • jupp
  • links
  • mercurial
  • mutt
  • plowshare
  • rdiff-backup
  • sshfs
  • vim

and maybe more... will be another 30+ files changed...

ymartin59
ymartin59 requested changes Jan 19, 2020
View reviewed changes
Copy link
Contributor

@ymartin59 ymartin59 left a comment

Choose a reason for hiding this comment

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

Really a large set of improvements and lots of file. Great clean up job !
Just found minor stuff as proposals

cross/tcl/Makefile Outdated
Comment on lines 2 to 3
PKG_VERS = 8.6.10
MAIN_VERS = 8.6
Copy link
Contributor

Choose a reason for hiding this comment

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

I propose an alternate way:

PKG_MAJOR_VERS = 8.6
PKG_VERS = $(PKG_MAJOR_VERS).10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer PKG_MAIN_VERS = 8.6
regarding semantic versioning "MAJOR" has a different meaning.
MAJOR_VERS = 8
MINOR_VERS = 6
PATCH_VERS = 10

Copy link
Contributor

Choose a reason for hiding this comment

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

OK with that. Idea is to avoid two distinct variables, with the risk to forget one at upgrade.

spk/tcl/Makefile Outdated
Comment on lines 2 to 3
SPK_VERS = 8.6.10
MAIN_VERS = 8.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Same proposal with variable reuse

This was referenced Jan 19, 2020
Merged
Closed
@hgy59
incorporate review
340119a 
- add ddrescue to synocli-disk
- move spk/ddrescue to diyspk/ddrescue
- update symlink creation to work with /bin/sh on older DSM versions
@ymartin59 ymartin59 merged commit b883d56 into SynoCommunity:master Jan 23, 2020
@hgy59 hgy59 deleted the symlinks branch January 23, 2020 17:56
@hgy59 hgy59 mentioned this pull request Jan 23, 2020
Closed
@hgy59 hgy59 mentioned this pull request Jan 20, 2021
Closed
AlexPresso pushed a commit to AlexPresso/spksrc that referenced this pull request Jan 30, 2025
@hgy59 @ymartin59
framework: add symlink creation support (SynoCommunity#3852)
4432cbf 
* PLIST cleanup: use documented types only (i.e. bin,lib,lnk,rsc)

- replace types etc,shr,etc. by rsc
- avoid types bin,lib for non strippable files

* cross fixes

- rmlint: install as executable
- ddrescue: add compiler flags removed by patch (add optimization)
- screen: remove chown from install, install without version postfix

* Enhance framework for symlinks

- add generation and removement of symlinks defined in Makefile
- create folders for links on demand
- make call_func of installer silent
- fix error logs for unexisting INST_VARIABLES folder

* update command line tools

- define symlinks in Makefiles
- remove service-setup.sh scripts used for link creation only
- remove BETA from diyspk packages
- use variable for optional description in synocli-disk and synocli-net
- update tmux icon
- make package icons square (detox, rmlint, testdisk)
- use unique gnu icon (less, ddrescue)
- ddrescue: remove pre DSM 6 install scripts (dsm-control.sh, installer.sh)

* Update changelog of synocli-file and synocli-disk

- add ddrescue to synocli-disk
- move spk/ddrescue to diyspk/ddrescue
- update symlink creation to work with /bin/sh on older DSM versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ymartin59 ymartin59 ymartin59 approved these changes

Assignees
No one assigned
Labels
framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hgy59 @ymartin59 @chickenandpork