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.

Sign up for GitHub

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

Jump to bottom

Fix executing with JDK headless #45

Merged
merged 1 commit into from
Jun 19, 2020
Merged

Fix executing with JDK headless #45

merged 1 commit into from
Jun 19, 2020

Conversation

ItaloYeltsin
Copy link
Contributor

Description:

Describe your changes in details.

Related Issue:

Motivation and Context:

Solves the problem with deploying totalcross java components from api to tcz when using Java Headless.

Benefited Devices:

  • Device: arm devices.
  • OS: for building yocto images.

How Has This Been Tested?

  • build passed by test made by OS System.

Tested Devices:

  • Device: Linux ARM devices

@otavio
Copy link
Member

otavio commented Jun 9, 2020

Tested on Yocto Project and it worked just fine.

Fixes: #27

LGTM.

otavio
otavio approved these changes Jun 9, 2020
View reviewed changes
Copy link
Member

@otavio otavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd annotate on the commit log a better explanation of the issue as well as it fixes #27

otavio
otavio requested changes Jun 17, 2020
View reviewed changes
Copy link
Member

@otavio otavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you succeed in reducing a lot the code involved in this processing. There are some commented code and it would be nice if it could be removed.

TotalCrossSDK/src/main/java/totalcross/ui/image/Image.java Outdated Show resolved Hide resolved
TotalCrossSDK/src/main/java/totalcross/ui/image/Image.java Outdated Show resolved Hide resolved
TotalCrossSDK/src/main/java/totalcross/ui/image/Image.java Outdated Show resolved Hide resolved
@otavio
Copy link
Member

otavio commented Jun 18, 2020

Can we re-test this? or should we wait?

@flsobral
Copy link
Member

It seems you succeed in reducing a lot the code involved in this processing. There are some commented code and it would be nice if it could be removed.

yeah, this new implementation uses only ImageIO, nothing from awt and media toolkit which are not always available on headless environment.
The dead code you mentioned before was intentional, but I wasn't sure about the best place to insert a comment explaining why the loop doesn't have a break condition.
In the end I decided to explain everything inside the catch clause but keeping the code for the break condition inside block comment for clarity.

@flsobral
Copy link
Member

Can we re-test this? or should we wait?

Yup, it's done. :)

@flsobral flsobral self-assigned this Jun 18, 2020
@flsobral flsobral linked an issue Jun 18, 2020 that may be closed by this pull request
When running the build for the TotalCross bytecode, we get exception if running on a headless mode #27
Closed
@otavio
Copy link
Member

otavio commented Jun 18, 2020

Are you intending to squash the changes? if so, please do and we can run a test here.

@flsobral flsobral force-pushed the bugfix/deploy-headless branch from ea21dd4 to 519ea03 Compare June 18, 2020 16:52
@flsobral
Copy link
Member

Are you intending to squash the changes? if so, please do and we can run a test here.

Done!

@flsobral
Fix the creation of TotalCross modules (tcz file) using Java headless
e8c50e3 
Deploy uses TotalCross' Image class to convert unsupported image
formats to PNG, and this process relied on classes from java.awt
and Toolkit, which may not be available on headless distros.

Changed implementation of ImageLoader to perform this conversion
using ImageIO instead, with some refactoring along the way for
clarity.

Fixes #27
@flsobral flsobral force-pushed the bugfix/deploy-headless branch from 519ea03 to e8c50e3 Compare June 18, 2020 17:50
@ItaloYeltsin
Copy link
Contributor Author

@otavio waiting for the test! =)

otavio
otavio approved these changes Jun 19, 2020
View reviewed changes
Copy link
Member

@otavio otavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, it worked fine.

LGTM.

@flsobral flsobral merged commit bd9fb51 into develop Jun 19, 2020
@otavio otavio deleted the bugfix/deploy-headless branch June 19, 2020 21:25
flsobral added a commit that referenced this pull request Jul 16, 2020
@ItaloYeltsin @flsobral
Fix the creation of TotalCross modules (tcz file) using Java headless (…
a2ca1da 
…#45)

Deploy uses TotalCross' Image class to convert unsupported image
formats to PNG, and this process relied on classes from java.awt
and Toolkit, which may not be available on headless distros.

Changed implementation of ImageLoader to perform this conversion
using ImageIO instead, with some refactoring along the way for
clarity.

Fixes #27

Co-authored-by: Fabio <flsobral@gmail.com>
flsobral added a commit that referenced this pull request Jul 24, 2020
@ItaloYeltsin @flsobral
Fix the creation of TotalCross modules (tcz file) using Java headless (…
f021411 
…#45)

Deploy uses TotalCross' Image class to convert unsupported image
formats to PNG, and this process relied on classes from java.awt
and Toolkit, which may not be available on headless distros.

Changed implementation of ImageLoader to perform this conversion
using ImageIO instead, with some refactoring along the way for
clarity.

Fixes #27

Co-authored-by: Fabio <flsobral@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@otavio otavio otavio approved these changes

Assignees

@flsobral flsobral

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

When running the build for the TotalCross bytecode, we get exception if running on a headless mode
3 participants
@ItaloYeltsin @otavio @flsobral