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

Split Makefile to be self-contained per directory - resubmit #553

Merged
merged 63 commits into from
Oct 5, 2021

Conversation

linuxkidd
Copy link
Collaborator

Re-submit for accidentally closed PR #545

I think this addresses all the comments and concerns from that earlier PR. Please let me know if you find anything else that needs attention.

Split the src/Makefile out to each directory where installation actions are needed:

  • config_repo
  • notification_images
  • scripts
  • src
  • Top-level

The Makefile in each sub-directory can be called individually and it'll perform the proper actions, independent of other directories.

The top-level Makefile calls all sub-dir's functions as needed.
Tweaked install.sh to use the top-level Makefile.

src/Makefile Show resolved Hide resolved
@linuxkidd
Copy link
Collaborator Author

Michael, in Makefile, you added
...
That code seems to be at odds with the title of the commit (Relocated allsky.sh back to top level, removed .repo version). The code puts allsky.sh in libexecdir and uses the .repo version. Are we keeping allsky.sh.repo or not? Where will allsky.sh exist?

@EricClaeys The .repo is a typo, I've fixed that in the next commit. The 'libexec' stuff is only triggered when using the PKGBUILD environment variable -- not the default action. It's for future package building convenience.

@EricClaeys
Copy link
Collaborator

config.sh.repo: Since config.sh is sourced after variables.sh, the following lines in config.sh.repo can use $ALLSKY_CONFIG instead of $ALLSKY_HOME/config:

CAMERA_SETTINGS_DIR="${ALLSKY_HOME}/config"
source "$ALLSKY_HOME/config/autocam.sh"

config_repo/Makefile: line 60: @sed -e "s|User=pi|User=allsky|g" -e "s|/home/pi/allsky|$(bindir)|g" allsky.service.repo > allsky.service Will changing the User to "allsky" break anything?
config_repo/Makefile: line 63: @echo -e "export ALLSKY_TMP=/tmp\nexport ALLSKY_CONFIG=$(DESTDIR)$(sysconfdir)/allsky\nexport ALLSKY_SCRIPTS=$(DESTDIR)$(libexecdir)\nexport ALLSKY_NOTIFICATION_IMAGES=$(DESTDIR)$(sharedir)\nexport ALLSKY_IMAGES=/home/allsky/Pictures/" > $(DESTDIR)$(sysconfdir)/profile.d/allsky.sh
Is that for possible future use? The locations aren't what we're currently using.

notification_images/Makefile: line 37: install -m 0644 *.jpg *.png $(DESTDIR)$(sharedir)/; \ I created PR 565 to add AllskyNotifications.psd which I used to create all the .jpg and .png files, so it should probably be copied as well.

@linuxkidd, @ckuethe,
since this PR has lots of changes to almost every file, can we test it on a Pi prior to merging it, both as a clean install and as an upgrade (whatever that means)? I suspect we missed a variable name change or location change or something...

Also, how do you put @ names in yellow? When I do an @name it doesn't have a yellow background.

@linuxkidd
Copy link
Collaborator Author

The yellow / brown or on the name is to highlight your own name so it's easier for you to spot.

Yes, I agree.. I'd much prefer you compile / test it. It runs great for me with a fresh install on Buster.

@EricClaeys
Copy link
Collaborator

I won't have access to my Pi for another week.

@ckuethe, are you able to install Michael's PR on your Pi and test it? I think if it works for 2 people it should be good to go.

@ckuethe
Copy link
Collaborator

ckuethe commented Oct 5, 2021

Yeah, I'm reading through it now and will set up a test machine tomorrow.

@linuxkidd
Copy link
Collaborator Author

Ok, I moved the variables.sh script back to $ALLSKY_HOME per Eric's request.

I also took the time to make an Uninstall function, including uninstall.sh

Makefile Outdated Show resolved Hide resolved
uninstall.sh Outdated Show resolved Hide resolved
@ckuethe ckuethe merged commit bee2624 into AllskyTeam:master Oct 5, 2021
@linuxkidd linuxkidd deleted the split-makefile branch October 5, 2021 23:20
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.

*** Waiting for you to fix this. Restart when done fixing. ***
3 participants