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

Provide installation via linuxbrew #537

Closed
daquinoaldo opened this issue Jun 15, 2019 · 34 comments
Closed

Provide installation via linuxbrew #537

daquinoaldo opened this issue Jun 15, 2019 · 34 comments
Assignees
Milestone

Comments

@daquinoaldo
Copy link

Hi! Thank you for your work, I really appreciate the new feature added to the skilion's version.
I would like to propose an easy way to install via homebrew.

Is your feature request related to a problem? Please describe.
It's a bit boring to install onedrive by building the source code manually.

Describe the solution you'd like
Linuxbrew is a package manager that works on all Linux distribution. Packages can be installed with brew install onedrive, and can be updated with brew upgrade. In my opinion, installing and upgrading via package manager is an easy way to keep this tool updated and to distribute the tool to more users.

Describe alternatives you've considered
Alternatives are disto-specific package manager (apt, apk, pacman, etc.) or no package manager at all.

Additional context

@norbusan
Copy link
Collaborator

Good idea, I will work on that. I have done some other brews, too ;-)

@norbusan norbusan self-assigned this Jun 15, 2019
@abraunegg abraunegg added the Feature Request Feature Request | Enhancement Request label Jun 15, 2019
@norbusan
Copy link
Collaborator

Here is a start: https://github.com/norbusan/homebrew-core/blob/onedrive/Formula/onedrive.rb
Installation fails because we try to chown root.users /var/log/onedrive, this needs to be either fakerooted or remove during installation in brew. Other todos are in the formula.

@norbusan
Copy link
Collaborator

@daquinoaldo If you can contribute, please send comments/patches/suggestions.

@abraunegg
Copy link
Owner

@daquinoaldo
Patches welcome to implement this feature request

@daquinoaldo
Copy link
Author

I'm sorry, I never created a brew formula nor I know how this tool is implemented, but I can test it when the formula is ready.

@daquinoaldo
Copy link
Author

Here is a start: https://github.com/norbusan/homebrew-core/blob/onedrive/Formula/onedrive.rb
Installation fails because we try to chown root.users /var/log/onedrive, this needs to be either fakeroot ed or remove during installation in brew. Other todos are in the formula.
@norbusan

I suppose this is the instruction that causes the problem: https://github.com/abraunegg/onedrive/blob/master/Makefile.in#L103.

I cannot find where DESTDIR, it seems to be undefined, indeed /var/log/onedrive exists.
Usually, homebrew doesn't install to root but to a local path. Maybe there could be a way to specify DESTDIR from homebrew formula? Installing onedrive to a user's path I think should solve the problem. I suppose that if we are able to install onedrive with make install instead of `sudo make install most of the work is done.

@norbusan
Copy link
Collaborator

Yes, it is this line. DESTDIR is normally undefined, but allows for staged installs. Not sure, but I guess that homebrew uses that for installation, too.

Concerning the chmod (via install) I have to think about it. I don't like it that we require root for installation at the moment. @abraunegg WDYT?

@norbusan
Copy link
Collaborator

Ok, I have it more or less ready. There are a few problems with the current code base (the --disable-XX arguments don't work - my fault!). After all of them have been fixed and a new release has been made, I will send another link for a formula. At the moment, installation fails badly ;-)

@norbusan
Copy link
Collaborator

Got it working now, submitted a PR to Homebrew (from which linuxbrew pulls): Homebrew/homebrew-core#40994

@abraunegg
Copy link
Owner

@norbusan
Would be worth updating install.md as well with applicable details once everything is working correctly

@abraunegg
Copy link
Owner

@norbusan
Based on the 'brew' feedback - it is best we get everything else fixed (#539) then release v2.3.5 for brew to build against.

Also they seem to have an issue with this being a fork ... not much I can do about it since the original maintainer is AWOL

@abraunegg
Copy link
Owner

@norbusan
Also the 'brew' compile fails:

src/monitor.d(11): Error: undefined identifier `IN_CLOSE_WRITE`
src/monitor.d(11): Error: undefined identifier `IN_CREATE`
src/monitor.d(11): Error: undefined identifier `IN_DELETE`
src/monitor.d(12): Error: undefined identifier `IN_MOVE`
src/monitor.d(12): Error: undefined identifier `IN_IGNORED`
src/monitor.d(12): Error: undefined identifier `IN_Q_OVERFLOW`
src/monitor.d(158): Error: undefined identifier `inotify_event`

This is probably due to the LDC version is too old:

checking for dmd... ldc2

Can we update ./configure to check for minimum versions:

  • DMD >= 2.083.1
  • LDC >= 1.11.0

@norbusan
Copy link
Collaborator

@abraunegg aah, good, you can test on real Mac hardware. I always have to do all the development on linuxbrew here :-(

I will look into the version dependencies.

Concerning fork: we could terminate the fork relation to the old onedrive, that is possible on github.

@abraunegg
Copy link
Owner

Concerning fork: we could terminate the fork relation to the old onedrive, that is possible on github.

I don't see this as a major issue atm ... only if 'brew' folk complain ...

@norbusan
Copy link
Collaborator

Agreed.

@abraunegg abraunegg added this to the v2.3.5 milestone Jun 16, 2019
@abraunegg
Copy link
Owner

@norbusan

I will look into the version dependencies.

Any update re this?

@norbusan
Copy link
Collaborator

No, sorry. I wait for a new release of onedrive, and then I will update the formula.

@abraunegg
Copy link
Owner

@norbusan
Ahh ok - I was thinking that the ./configure script could be updated rather than the formula - so that the check is done not just for 'brew' but for all

@norbusan
Copy link
Collaborator

@abraunegg aahh, ok, indeed, that is the better idea. I think about it, but this needs some parsing of the output..

@norbusan
Copy link
Collaborator

HI @abraunegg I wrote code for configure.ac. The hard part was the version number comparison in a way that even a plain POSIX shell like dash can work with it.
I hope it works out with other shells, too. I tried bash and dash.
#545

The version check can be overridden with --disable-version-check

@norbusan
Copy link
Collaborator

@abraunegg one strange thing: it seems that homebrew has ldc 1.15.0, so by far new enough. It is surprising that it does not compile then...

@abraunegg
Copy link
Owner

abraunegg commented Jun 18, 2019

@norbusan
Will have a look at the build logs in a sec - did this get updated - Homebrew/homebrew-core#40994 ?

@norbusan
Copy link
Collaborator

No, what should I update there? I will updated when 2.3.5 is released. ATM there is not much we can change, right?

@abraunegg
Copy link
Owner

@norbusan
Correct - need to get the x32 builds working right

@norbusan
Copy link
Collaborator

@abraunegg I have updated the PR/branch with 2.3.5, and removed the patch. Now the CI testing only complains about

  • Dependency 'curl-openssl' may be unnecessary as it is provided by macOS; try to build this formula without it.
  • Dependency 'sqlite' may be unnecessary as it is provided by macOS; try to build this formula without it.
  • GitHub fork (not canonical repository)

Do you have a Mac? Can you try building without the deps?

Does that work? (I can only test linuxbrew)

@abraunegg
Copy link
Owner

I was just using the build logs from the brew github, tho I do have a Mac VM I use when testing other things - tho its rather old. Will try and see what happens

@norbusan
Copy link
Collaborator

@abraunegg ok, forget it. I simply remove the deps and see whether it builds on github ;-)

@norbusan
Copy link
Collaborator

I also added libnotify dependency - hopefully that fixes the build errors.

@norbusan
Copy link
Collaborator

@abraunegg ok ... now I get it.

We cannot have onedrive on Mac, there is no inotify, this is a kernel question.

So we cannot go the route via including onedrive in normal homebrew and thus enter into linuxbrew. I close the pull request.

I need to check how we could go on with linuxbrew support only.

@abraunegg abraunegg modified the milestones: v2.3.5, v2.3.6 Jun 20, 2019
@steve3p0
Copy link

I would like to give this a big thumbs up and a big VOTE for YES PLEASE DO THIS!!!

Thank you guys so much for all the work that you put into this. BRAVO!!! I am eagerly waiting for an easy way to install this on Ubuntu 18.04.

@norbusan
Copy link
Collaborator

@steve3p0 I already sent a PR to linuxbrew, but there is no answer. I'll wait a bit longer, and if that doesn't work, we will provide some private tap somewhere.

@abraunegg
Copy link
Owner

As PR submitted to linuxbrew (https://github.com/Homebrew/linuxbrew-core/pull/13776) & it just needs review - closing this issue tracker as not a bug, and basically waiting on linuxbrew to include.

@abraunegg abraunegg removed the Feature Request Feature Request | Enhancement Request label Jul 14, 2019
@daquinoaldo
Copy link
Author

Thank you!

@lock
Copy link

lock bot commented Aug 13, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Aug 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants