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

Add man page and update the Makefile #80

Merged
merged 2 commits into from
Sep 2, 2016

Conversation

JR1994
Copy link

@JR1994 JR1994 commented Jul 4, 2016

I am pretty new to git. I made some changes and placed pull requests to the upstream repo. Since I am cloning yours I thought you would want to take a look at them as well. I have made a man page for mbpfan and updated the Makefile. The Makefile will use the install command instad of cp. It will also install the systemd .service file, the man page, and the README file. These changes will also make packaging easier.

Herminio Hernandez Jr added 2 commits July 3, 2016 18:53
…rvice file, and README will also be installed. This change will allow the building of .deb packages.
@dgraziotin
Copy link
Collaborator

Thank you very much for this. It will take a while before I can review this PR. Please have some patience.

@JR1994
Copy link
Author

JR1994 commented Jul 4, 2016

No worries this app has been a great for my MacBook Pro running Debian.

Sent from my iPhone

On Jul 4, 2016, at 12:01 AM, Daniel Graziotin notifications@github.com wrote:

Thank you very much for this. It will take a while before I can review this PR. Please have some patience.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@JR1994
Copy link
Author

JR1994 commented Aug 8, 2016

Any update on this request?

@dgraziotin
Copy link
Collaborator

I'll likely look at it during the last days of August. Sorry, big deadlines at work!

Daniel Graziotin
@dgraziotin
ineed.coffee

On 08 Aug 2016, at 02:25, JR1994 notifications@github.com wrote:

Any update on this request?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@dgraziotin
Copy link
Collaborator

@JR1994 I have finally started looking at your PR. I am not a Makefile expert (at all), and was wondering what would be the consequences of a Makefile that incorporates systemd for systems that do not use systemd. Do you have any clue?

@dgraziotin dgraziotin mentioned this pull request Aug 29, 2016
@JR1994
Copy link
Author

JR1994 commented Aug 29, 2016

It will only install the systemd.service file and attempt to launch it. One
will still need to run systemctl enable mbpfan.service and systemctl start
mbpfan.service

On Mon, Aug 29, 2016 at 1:19 AM, Daniel Graziotin notifications@github.com
wrote:

@JR1994 https://github.com/JR1994 I have finally started looking at
your PR. I am not a Makefile expert (at all), and was wondering what would
be the consequences of a Makefile that incorporates systemd
8e68a2b#diff-b67911656ef5d18c4ae36cb6741b7965R79
for systems that do not use systemd. Do you have any clue?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFHmIAU-kllDmqK5CF015MVtoDWIwlc_ks5qkpX8gaJpZM4JEAnQ
.

@dgraziotin
Copy link
Collaborator

That's right. What about those distros that do not use systemd?

Daniel Graziotin
@dgraziotin
ineed.coffee

On 30 Aug 2016, at 00:50, JR1994 notifications@github.com wrote:

It will only install the systemd.service file and attempt to launch it. One
will still need to run systemctl enable mbpfan.service and systemctl start
mbpfan.service

On Mon, Aug 29, 2016 at 1:19 AM, Daniel Graziotin notifications@github.com
wrote:

@JR1994 https://github.com/JR1994 I have finally started looking at
your PR. I am not a Makefile expert (at all), and was wondering what would
be the consequences of a Makefile that incorporates systemd
8e68a2b#diff-b67911656ef5d18c4ae36cb6741b7965R79
for systems that do not use systemd. Do you have any clue?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFHmIAU-kllDmqK5CF015MVtoDWIwlc_ks5qkpX8gaJpZM4JEAnQ
.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@JR1994
Copy link
Author

JR1994 commented Aug 30, 2016

There are fewer and fewer that are non-systemd. Gentoo still uses OpenRC, but you can also use systemd. Anything Debian based after Jessie is on systemd. Linux Mint 18 is now on sustemd. Ubuntu has been on systemd since 15.04. More and more systemd is becoming the standard.

Sent from my iPhone

On Aug 29, 2016, at 10:00 PM, Daniel Graziotin notifications@github.com wrote:

That's right. What about those distros that do not use systemd?

Daniel Graziotin
@dgraziotin
ineed.coffee

On 30 Aug 2016, at 00:50, JR1994 notifications@github.com wrote:

It will only install the systemd.service file and attempt to launch it. One
will still need to run systemctl enable mbpfan.service and systemctl start
mbpfan.service

On Mon, Aug 29, 2016 at 1:19 AM, Daniel Graziotin notifications@github.com
wrote:

@JR1994 https://github.com/JR1994 I have finally started looking at
your PR. I am not a Makefile expert (at all), and was wondering what would
be the consequences of a Makefile that incorporates systemd
8e68a2b#diff-b67911656ef5d18c4ae36cb6741b7965R79
for systems that do not use systemd. Do you have any clue?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFHmIAU-kllDmqK5CF015MVtoDWIwlc_ks5qkpX8gaJpZM4JEAnQ
.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@dgraziotin
Copy link
Collaborator

Sure, but there are still distros not using it, e.g., Slackware (on which I developed mbpfan initially) and older versions of Ubuntu, Debian, Fedora, etc.. Mbpfan aims to be distro independent.

Now, I am wondering what would happen when one does a make install on a system where systemd is not present. Would it fail? Would it just print an error message?

Shall we instead go for a make systemdinstall instead?

@JR1994
Copy link
Author

JR1994 commented Aug 30, 2016

On 08/30, Daniel Graziotin wrote:

Sure, but there are still distros not using it, e.g., Slackware (on which I developed mbpfan initially) and older versions of Ubuntu, Debian, Fedora, etc.. Mbpfan aims to be distro independent.

Now, I am wondering what would happen when one does a make install on a system where systemd is not present. Would it fail? Would it just print an error message?

I think it will just create the /lib/systemd directory and that is all. However I am thinking that we may not need that line. In Debian they use dh_auto and dh_install to overide the upstream Makefile. I think gentoo ebuilds and Arch PKGBUILDs do the same. So the downstream distro maintainers can decide how to install that portion. Your thoughts?

Thanks
Herminio

@dgraziotin
Copy link
Collaborator

That would definitely solve the doubts. I was also wondering if it was a job for packages to install distro-dependent files. If that sounds OK to you, I will merge your parts without the systemd related code.

@JR1994
Copy link
Author

JR1994 commented Aug 30, 2016

I am not 100% sure but think so.

Sent from my iPhone

On Aug 30, 2016, at 1:10 AM, Daniel Graziotin notifications@github.com wrote:

That would definitely solve the doubts. I was also wondering if it was a job for packages to install distro-dependent files. If that sounds OK to you, I will merge your parts without the systemd related code.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

dgraziotin added a commit that referenced this pull request Aug 31, 2016
@dgraziotin
Copy link
Collaborator

Ok, I created a branch with your edits and some small modifications (one of which is deleting the -Wall option from the Makefile, as this is a separate issue to be addressed). Does it look OK to you? Feel free to add yourself to the AUTHORS file!

@JR1994
Copy link
Author

JR1994 commented Aug 31, 2016

Looks good. Thanks for the offer!

Herminio

On Wed, Aug 31, 2016 at 12:47 AM, Daniel Graziotin <notifications@github.com

wrote:

Ok, I created a branch
https://github.com/dgraziotin/mbpfan/tree/JR1994-master with your edits
and some small modifications (one of which is deleting the -Wall option
from the Makefile, as this is a separate issue to be addressed). Does it
look OK to you? Feel free to add yourself to the AUTHORS file!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#80 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFHmIIdSU5BzuvAMhcd4uPGSVVyGOmFSks5qlTGRgaJpZM4JEAnQ
.

@dgraziotin dgraziotin merged commit 8e68a2b into linux-on-mac:master Sep 2, 2016
@dgraziotin
Copy link
Collaborator

@JR1994 thanks again for this!

@JR1994
Copy link
Author

JR1994 commented Sep 2, 2016

You are welcome

Sent from my iPhone

On Sep 2, 2016, at 1:07 AM, Daniel Graziotin notifications@github.com wrote:

@JR1994 thanks again for this!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

wezm added a commit to wezm/mbpfan that referenced this pull request Jan 16, 2017
sudo dmidecode -s system-product-name
Macmini5,3

uname -a
Linux elementaty-mini 4.4.0-59-generic linux-on-mac#80-Ubuntu SMP Fri Jan 6 17:47:47 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

sudo make tests
make install
make[1]: Entering directory '/home/wmoore/Source/mbpfan'
make
make[2]: Entering directory '/home/wmoore/Source/mbpfan'
Linking...
g++  -g -lm  src/settings.o src/daemon.o src/mbpfan.o src/main.o src/minunit.o src/strmap.o -lm -o bin/mbpfan
make[2]: Leaving directory '/home/wmoore/Source/mbpfan'
install -d /usr/sbin
install -d /etc
install -d /lib/systemd/system
install -d /usr/share/doc/mbpfan
install bin/mbpfan /usr/sbin
install -m644 mbpfan.conf /etc
install -m644 README.md /usr/share/doc/mbpfan
install -d /usr/share/man/man8
install -m644 mbpfan.8.gz /usr/share/man/man8

******************
INSTALL COMPLETED
******************

A configuration file has been copied (might overwrite existing file) to /etc/mbpfan.conf.
See README.md file to have mbpfan automatically started at system boot.

Please run the tests now with the command
   sudo make tests

make[1]: Leaving directory '/home/wmoore/Source/mbpfan'
/usr/sbin/mbpfan -f -v -t
Starting the tests..
It is normal for them to take a bit to finish.
Detected kernel version: 4
Detected kernel minor revision: 4
Detected kernel version: 4
Detected kernel minor revision: 4
Using new sensor path for kernel >= 3.0.15
Found hwmon path at /sys/devices/platform/coretemp.0/hwmon/hwmon0/temp
Found 5 sensors
Found 1 fans
Detected kernel version: 4
Detected kernel minor revision: 4
Using new sensor path for kernel >= 3.0.15
Found hwmon path at /sys/devices/platform/coretemp.0/hwmon/hwmon0/temp
Found 5 sensors
Testing the _supplied_ mbpfan.conf (not the one you are using)..
ALL TESTS PASSED
Tests run: 8
wezm added a commit to wezm/mbpfan that referenced this pull request Jan 16, 2017
sudo dmidecode -s system-product-name
Macmini5,3

uname -a
Linux elementaty-mini 4.4.0-59-generic linux-on-mac#80-Ubuntu SMP Fri Jan 6 17:47:47 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

sudo make tests
make install
make[1]: Entering directory '/home/wmoore/Source/mbpfan'
make
make[2]: Entering directory '/home/wmoore/Source/mbpfan'
Linking...
g++  -g -lm  src/settings.o src/daemon.o src/mbpfan.o src/main.o src/minunit.o src/strmap.o -lm -o bin/mbpfan
make[2]: Leaving directory '/home/wmoore/Source/mbpfan'
install -d /usr/sbin
install -d /etc
install -d /lib/systemd/system
install -d /usr/share/doc/mbpfan
install bin/mbpfan /usr/sbin
install -m644 mbpfan.conf /etc
install -m644 README.md /usr/share/doc/mbpfan
install -d /usr/share/man/man8
install -m644 mbpfan.8.gz /usr/share/man/man8

******************
INSTALL COMPLETED
******************

A configuration file has been copied (might overwrite existing file) to /etc/mbpfan.conf.
See README.md file to have mbpfan automatically started at system boot.

Please run the tests now with the command
sudo make tests

make[1]: Leaving directory '/home/wmoore/Source/mbpfan'
/usr/sbin/mbpfan -f -v -t
Starting the tests..
It is normal for them to take a bit to finish.
Detected kernel version: 4
Detected kernel minor revision: 4
Detected kernel version: 4
Detected kernel minor revision: 4
Using new sensor path for kernel >= 3.0.15
Found hwmon path at /sys/devices/platform/coretemp.0/hwmon/hwmon0/temp
Found 5 sensors
Found 1 fans
Detected kernel version: 4
Detected kernel minor revision: 4
Using new sensor path for kernel >= 3.0.15
Found hwmon path at /sys/devices/platform/coretemp.0/hwmon/hwmon0/temp
Found 5 sensors
Testing the _supplied_ mbpfan.conf (not the one you are using)..
ALL TESTS PASSED
Tests run: 8
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.

2 participants