Bug 1421603 - Container Review Request: toolchain - Platform for building C and C++ applications
Summary: Container Review Request: toolchain - Platform for building C and C++ applica...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tomas Tomecek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-02-13 09:00 UTC by Honza Horak
Modified: 2017-02-23 16:28 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-02-23 16:28:54 UTC
Type: ---
Embargoed:
ttomecek: fedora-review+


Attachments (Terms of Use)

Description Honza Horak 2017-02-13 09:00:29 UTC
Dockerfile URL: https://hhorak.fedorapeople.org/toolchain-docker/Dockerfile
Other files URL: https://hhorak.fedorapeople.org/toolchain-docker/

Description: This container image provides a platform for building C and C++ applications. It is a Fedora alternative to Developer Toolset Toolchain from Red Hat Software Collections.

Fedora Account System Username: hhorak

Comment 1 Tomas Tomecek 2017-02-13 15:10:29 UTC
Well, since we don't have any `fedora-review` nor `rpmlint`, I'll just inline-comment:

> io.k8s.display-name="Fedora alternative to Developer Toolset Toolchain"

I would name it as "Fedora variant of software collection Devel..."

Not sure about capitals.


> ADD contrib/bin/usage /usr/local/bin/usage

Why don't you add it to /opt/app-root? (nitpick)

I would also prefer COPY (since ADD may have side effects).


> https://hhorak.fedorapeople.org/toolchain-docker/contrib/etc/scl_enable

This seems redundant.


I would also like to see some documentation with better examples: how would I use the container to compile my code?

Comment 2 Honza Horak 2017-02-14 15:29:15 UTC
Thanks for the suggestions!

(In reply to Tomas Tomecek from comment #1)
> > io.k8s.display-name="Fedora alternative to Developer Toolset Toolchain"
> 
> I would name it as "Fedora variant of software collection Devel..."

Hm, what about "Fedora variant of Developer Toolset's Toolchain from Software Collections"
 
> > ADD contrib/bin/usage /usr/local/bin/usage
> 
> Why don't you add it to /opt/app-root? (nitpick)

Reported in https://github.com/sclorg/devtoolset-container/issues/15, but will fix in the Fedora one.

> I would also prefer COPY (since ADD may have side effects).

Ack.

> > https://hhorak.fedorapeople.org/toolchain-docker/contrib/etc/scl_enable
> 
> This seems redundant.

Yes, this is not relevant in Fedora.

> I would also like to see some documentation with better examples: how would
> I use the container to compile my code?

Good question, we should have it written better in the usage and provide some nice README.md, which is now missing entirely. Will work on it with Marek (original author of https://github.com/sclorg/devtoolset-container/tree/master/4-toolchain)

Comment 3 Honza Horak 2017-02-15 08:28:52 UTC
I believe I've fixed all issues mentioned above, new sources are still on the same place:

Dockerfile URL: https://hhorak.fedorapeople.org/toolchain-docker/Dockerfile
Other files URL: https://hhorak.fedorapeople.org/toolchain-docker/

Comment 4 Tomas Tomecek 2017-02-15 12:12:35 UTC
One last nit (not blocking the transition): the readme is not being added into the image. Please make this change in dist-git.

Otherwise I'm fine with the image, well done!

Comment 5 Tomas Tomecek 2017-02-15 12:48:54 UTC
Also:

$ docker run tt/maria
/usr/bin/container-entrypoint: line 5: /usr/bin/usage: Permission denied

You would run into it anyway. It doesn't have "+x".

Comment 6 Ralph Bean 2017-02-15 19:28:54 UTC
Package request has been approved: https://admin.stg.fedoraproject.org/pkgdb/package/modules/toolchain

Comment 7 Ralph Bean 2017-02-15 19:42:53 UTC
Package request has been approved: https://admin.stg.fedoraproject.org/pkgdb/package/modules/postfix

Comment 8 Ralph Bean 2017-02-15 19:43:44 UTC
Package request has been approved: https://admin.stg.fedoraproject.org/pkgdb/package/docker/postfix

Comment 9 Honza Horak 2017-02-16 12:22:59 UTC
(In reply to Tomas Tomecek from comment #4)
> One last nit (not blocking the transition): the readme is not being added
> into the image. Please make this change in dist-git.

Sure, will do

> $ docker run tt/maria
> /usr/bin/container-entrypoint: line 5: /usr/bin/usage: Permission denied

I thought I fixed it already, but probably did not upload to the fedorapeople.org. The test should also catch this.

Thanks Tomas!!

Comment 10 Gwyn Ciesla 2017-02-16 13:36:32 UTC
Package request has been denied with the reason: Please request in rpms namespace, not docker.

Comment 11 Honza Horak 2017-02-16 14:26:58 UTC
This is a container review, I believe docker namespace is correct.

Comment 12 Honza Horak 2017-02-16 14:28:10 UTC
Following https://fedoraproject.org/wiki/Container:Review_Process

Comment 13 Gwyn Ciesla 2017-02-16 14:31:04 UTC
Right you are, please resubmit.

Comment 14 Honza Horak 2017-02-16 15:39:18 UTC
Ok, done.

Comment 15 Gwyn Ciesla 2017-02-16 16:01:56 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/docker/toolchain

Comment 16 Tomas Tomecek 2017-02-17 13:19:15 UTC
Since you already built the image, can we close this?

Comment 17 Honza Horak 2017-02-23 16:28:54 UTC
Yes, we can. Thanks for reminding.


Note You need to log in before you can comment on or make changes to this bug.