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

Build with gmake < 4 broken: missing separator #9387

Closed
jengelh opened this issue May 4, 2016 · 15 comments
Closed

Build with gmake < 4 broken: missing separator #9387

jengelh opened this issue May 4, 2016 · 15 comments
Labels

Comments

@jengelh
Copy link

@jengelh jengelh commented May 4, 2016

Please follow the guide below

  • You will be asked some questions and requested to provide some information, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your issue (like that [x])
  • Use Preview tab to see how your issue will actually look like

Make sure you are using the latest version: run youtube-dl --version and ensure your version is 2016.05.01. If it's not read this FAQ entry and update. Issues with outdated version will be rejected.

  • I've verified and I assure that I'm running youtube-dl 2016.05.01

Before submitting an issue make sure you have:

  • At least skimmed through README and most notably FAQ and BUGS sections
  • Searched the bugtracker for similar issues including closed ones

What is the purpose of your issue?

  • Bug report (encountered problems with youtube-dl)
  • Site support request (request for adding support for a new site)
  • Feature request (request for a new functionality)
  • Question
  • Other

When buliding under openSUSE 13.1 with (GNU) make-3.82, or SLES 11 with GNU make-3.81, the build fails:

[   33s] + /usr/bin/mkdir /home/abuild/rpmbuild/BUILDROOT/youtube-dl-2016.05.01-112.1.x86_64
[   33s] + cd youtube-dl
[   33s] + perl -i -pe '
[   33s]         s{^PREFIX\ \?=\ /usr/local}{PREFIX ?= /usr};
[   33s]         s{^BINDIR\ \?=\ \$\(PREFIX\)/bin}{BINDIR\ \?=\ /usr/bin};
[   33s]         s{^MANDIR\ \?=\ \$\(PREFIX\)/man}{MANDIR\ \?=\ /usr/share/man};
[   33s]         s{^SHAREDIR\ \?=\ \$\(PREFIX\)/share}{SHAREDIR\ \?=\ /usr/share};' Makefile
[   33s] + exit 0
[   33s] Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.3EufIx
[   33s] + umask 022
[   33s] + cd /home/abuild/rpmbuild/BUILD
[   33s] + cd youtube-dl
[   33s] + make DESTDIR=/home/abuild/rpmbuild/BUILDROOT/youtube-dl-2016.05.01-112.1.x86_64 install
[   33s] Makefile:15: *** missing separator.  Stop.

It appears that the variable != value syntax as introduced by commit 3ff63fb is not supported with make 3.x, but only 4.0 and up.

@jaimeMF jaimeMF added the build/update label May 4, 2016
@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented May 4, 2016

I don't know of other way to make it compatible with BSD make, so we'll have to think something. Note that gmake 4.0 was released in 2013, so it's not something really new.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented May 4, 2016

The following syntax:

SYSCONFDIR = $(shell if [ $(PREFIX) = /usr -o $(PREFIX) = /usr/local ]; then echo /etc; else echo $(PREFIX)/etc; fi)

Works on GNU make 4.1, too. Does it work on GNU make 3.81 and/or BSD make?

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented May 4, 2016

No, $(shell ...) is only available in GNU make.
I can be changed to

SYSCONFDIR = $$(if [ $(PREFIX) = /usr -o $(PREFIX) = /usr/local ]; then echo /etc; else echo $(PREFIX)/etc; fi)

but that means it gets executed each time you write $(SYSCONFDIR).

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented May 5, 2016

but that means it gets executed each time you write $(SYSCONFDIR).

If we want to support GNU make 3.x as well as BSD make it's inevitable. And it seems harmless to youtube-dl as the Makefile is quite small.

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented May 5, 2016

@jengelh it should just work, but could you check if it works with this change in GNU make 3.x?

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 5, 2016

@bdeyal Could you change the offending line:

SYSCONFDIR != if [ $(PREFIX) = /usr -o $(PREFIX) = /usr/local ]; then echo /etc; else echo $(PREFIX)/etc; fi

To the new syntax proposed in #9387 (comment) and check whether the old make on CentOS works or not?

@bdeyal
Copy link

@bdeyal bdeyal commented Oct 5, 2016

Works OK.

I use master with git hash 0a33bb2

changed 2 locations. First is the line you gave above (line 15) and second at line 93:
_EXTRACTOR_FILES = $$(find youtube_dl/extractor -iname '*.py' -and -not -iname 'lazy_extractors.py')

I tested a single download, works OK.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 5, 2016

Thanks. I'll check the changes on BSD make before commiting.

@bdeyal
Copy link

@bdeyal bdeyal commented Oct 5, 2016

It works fine with NetBSD make too (version 20150910, called bmake in Fedora) on both versions (before and after change in Makefile)

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 5, 2016

Oh thanks! Which Fedora version did you test with? And which version of make does CentOS comes with, 3.81 or 3.82?

@bdeyal
Copy link

@bdeyal bdeyal commented Oct 5, 2016

CentOS 7.2 comes with make 3.82
NetBSD bmake was built on CentOS from a Fedora 24 source rpm. Tested on CentOS, not Fedora.

Took the source rpm from here:
http://koji.fedoraproject.org/koji/buildinfo?buildID=716769

@yan12125 yan12125 closed this in 831a34c Oct 6, 2016
@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 6, 2016

Thanks for helping on tests. Fixed.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 7, 2016

Reverted my last change as breaking lazy extractors. Sorry for not having checked everything well.

@yan12125 yan12125 reopened this Oct 7, 2016
@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented Oct 8, 2016

Then I don't think it can be made to support both old gmake versions and bmake. I don't mind dropping support for bmake, I just thought it would be nice to have it.

@yan12125 yan12125 closed this in 8204c73 Oct 9, 2016
@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Oct 9, 2016

OK. Dropped BSD make support as installing gmake on *BSD systems seems quite easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.