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

Makefile improvement #6576

Closed
wants to merge 6 commits into from
Closed

Makefile improvement #6576

wants to merge 6 commits into from

Conversation

dudebout
Copy link
Contributor

Rebased pull request #6435 (which contains #6434 and the split echo statements).
Removed the prefix of newly introduced commands.

The echo command does not understand the -n argument on Mac OS X.
This is due to the fact that:

  - Makefile calls /bin/sh to execute a command
  - in Mac OS X, /bin/sh is a link to bash
  - in Mac OS X, bash is compiled with --enable-strict-posix-default

Therefore, the echo command does not have the -n argument.

printf on the other hand is a builtin function that works well on Mac OS X and Linux.
The change introduced in 5bd8cdc was:

  - partial, as not all the command were translated
  - preventing the use of a global npm installation
A user can use the following command to determine where to copy the resulting css file:

    $ BOOTSTRAP=<my-path> make
@fat
Copy link
Member

fat commented Feb 6, 2013

this reintroduces globals which we don't want

@fat fat closed this Feb 6, 2013
@dudebout
Copy link
Contributor Author

dudebout commented Feb 6, 2013

What do you mean by globals? If you mean that it uses the packages installed globally by npm, this is not the case. I use the path variable instead of hard coding the values. See #10a83b1.

@notbrain
Copy link

notbrain commented Feb 9, 2013

Thanks for these makefile improvements, made mine work.

@cvrebert
Copy link
Collaborator

cvrebert commented Feb 9, 2013

@fat: README.md's instructions currently say to install globally (-g)...

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.

5 participants