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

Depend on empy python module #114

Closed
wants to merge 1 commit into from

Conversation

srevinsaju
Copy link
Member

Fix makefile dependency on /usr/bin/empy for Linux distributions
which do not package a empy binary executable.

@srevinsaju
Copy link
Member Author

@chimosky @quozl

@quozl
Copy link
Contributor

quozl commented Mar 4, 2020

Thanks. Reviewed. Good progress, but unfinished. Please change the remaining references to $(EMPY).

@srevinsaju
Copy link
Member Author

@quozl fixed it. No more EMPY remains

@quozl
Copy link
Contributor

quozl commented Mar 5, 2020

@srevinsaju, thanks. Reviewed.

@quozl
Copy link
Contributor

quozl commented Mar 5, 2020

Build stalls because python is reading from terminal. Reproducer;

$ cd gtk3/theme
$ python -m em - -p $ -D scaling=72 -D gtk=3.24.5 gtk-widgets.css.em

@quozl
Copy link
Contributor

quozl commented Mar 9, 2020

Hey, @chimosky, looks like we may have lost forward motion on this. Would you be able to fix it and test on Fedora? In removing < $(EMPY) the standalone - should also be removed. I'll test on Debian and Ubuntu as part of release.

Fix makefile dependency on /usr/bin/empy for Linux distributions which do not package a empy binary executable.

Co-authored-by: Ibiam Chihurumnaya <ibiamchihurumnaya@gmail.com>
@quozl
Copy link
Contributor

quozl commented Mar 10, 2020

On Ubuntu 19.04, tested 7a4935f; from a clean repository, with autogen, make, make install, and start Sugar.

What of test result on Fedora?

@chimosky
Copy link
Member

Tested, builds as expected.

Logs

There's a downstream issue but it doesn't affect this.

@quozl
Copy link
Contributor

quozl commented Mar 10, 2020

That's odd. Your logs don't match this pull request; the output "checking for empy..." and "checking for empy3..." should have been removed as a result of AC_PATH_PROGS being removed. Could you please check again that you are testing with 7a4935f?

@srevinsaju
Copy link
Member Author

I am extremely sorry, I am away for a few days from programming. I will get back to this PR on Friday / Saturday.

I assume there was a hyphen character between em and the -p on the sugar PR too. I guess that should have also failed. This did not fail particularly, but I didn't test this line explicitly

python -m em - -p $ -D scaling=72 -D gtk=3.24.5 gtk-widgets.css.em

I don't have a desktop to test this. Seems like bash is too vast,

@chimosky
Copy link
Member

That's odd. Your logs don't match this pull request; the output "checking for empy..." and "checking for empy3..." should have been removed as a result of AC_PATH_PROGS being removed. Could you please check again that you are testing with 7a4935f?

Yeah I was testing with 7a4935f.

Tested again; builds as expected.

Logs;

@chimosky
Copy link
Member

I am extremely sorry, I am away for a few days from programming. I will get back to this PR on Friday / Saturday.

It's okay, it's been fixed.

I assume there was a hyphen character between em and the -p on the sugar PR too. I guess that should have also failed. This did not fail particularly, but I didn't test this line explicitly

There's quite a few differences between both changes.

@quozl
Copy link
Contributor

quozl commented Mar 12, 2020

Thanks for testing. I've fixed it in Sugar again, as make by itself outside any packaging wrapping did hang. sugarlabs/sugar@5d5c001

@quozl
Copy link
Contributor

quozl commented Mar 12, 2020

74d983e merged by hand.

@quozl quozl closed this Mar 12, 2020
@srevinsaju
Copy link
Member Author

Ok, I came back to continue my work on this PR, and found out this PR's closed.
Just for my knowledge, is the - sign force empy to read from the terminal?
I can see that when - was used with empy and python3 -m` has the same effect.
I may be wrong, but it might help me in future. Thanks

@quozl
Copy link
Contributor

quozl commented Mar 12, 2020

Sure. - as an option means to read from stdin. Unless stdin has been redirected, that means the stdin of the parent shell, such as bash. This will usually be a terminal, unless make is being run by some other script or program. In removing the stdin redirect < $(EMPY) it is also necessary to remove the - that causes it to be used. It would have been obvious in testing using make, because it would stall waiting for an end of file mark from the terminal, which is a ctrl+d in Linux terminals.

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.

None yet

3 participants