Skip to content

Conversation

@razvand
Copy link
Contributor

@razvand razvand commented Jul 8, 2021

Add Unikraft files for OpenJPEG

razvand and others added 3 commits July 8, 2021 11:22
Signed-off-by: Razvan Deaconescu <[email protected]>
@razvand razvand requested a review from mandrei12 July 8, 2021 08:25
@razvand razvand self-assigned this Jul 8, 2021
Copy link

@mandrei12 mandrei12 left a comment

Choose a reason for hiding this comment

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

Everything is okay.

Reviewed-by: Andrei Mutu [email protected]

@razvand razvand assigned nderjung and unassigned razvand Nov 29, 2021
@razvand razvand added the enhancement New feature or request label Nov 29, 2021
Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the initial port! Please see requested changes inline.

Comment on lines +1 to +11
Maintainers List
================

For notes on how to read this information, please refer to `MAINTAINERS.md` in
the main Unikraft repository.

LCMS-UNIKRAFT
M: Esteban Martinez <[email protected]>
M: Xavier Peralra <[email protected]>
L: [email protected]
F: *
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this file, it is now deprecated. :) We will add names to our governance repository instead.

Comment on lines +2 to +3
bool "libopenjpeg - image compression standard"
default n
Copy link
Member

Choose a reason for hiding this comment

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

- bool "libopenjpeg - image compression standard"
+ bool "libopenjpeg: Image compression standard"

Comment on lines +33 to +35
#
# THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
#
Copy link
Member

Choose a reason for hiding this comment

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

Please remove these lines. Please see this explanation as to why.

# Original sources
################################################################################
LIBOPENJPEG_VERSION=2.3.1
LIBOPENJPEG_URL=https://github.com/python-pillow/pillow-depends/raw/master/openjpeg-$(LIBOPENJPEG_VERSION).tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a weird place to get the archive from. IS there anything else more official?

Maybe https://github.com/uclouvain/openjpeg?

Also, it does not resolve and the fetch stage fails:

tar (child): /usr/src/unikraft/apps/helloworld/build/libopenjpeg/openjpeg-2.3.1.tar.gz: Cannot open: No such file or directory
tar (child): Error is not recoverable: exiting now
tar: Child returned status 2
tar: Error is not recoverable: exiting now

Comment on lines +115 to +123
# Run ./configure
$(LIBOPENJPEG_EXTRACTED)/config.status: $(LIBOPENJPEG_BUILD)/.origin
$(call verbose_cmd,CONFIG,libopenjpeg: $(notdir $@), \
cd $(LIBOPENJPEG_EXTRACTED) && mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Release)

LIBOPENJPEG_PREPARED_DEPS = \
$(LIBOPENJPEG_EXTRACTED)/config.status \

$(LIBOPENJPEG_BUILD)/.prepared: $(LIBOPENJPEG_PREPARED_DEPS)
Copy link
Member

Choose a reason for hiding this comment

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

These step needs to be removed. What is required from the configure step are the any configuration header files which need to be explicitly created and placed within a top-level include/ directory of this repo. The directory and corresponding header file should be referenced via LIBOPENJPEG_COMMON_INCLUDES. It's options need be checked and must correspond with what is compatible with Unikraft.

Also this step fails for me entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: No status
Status: 🧊 Icebox
Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants