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

CI: initial commit #69

Closed
wants to merge 14 commits into from
Closed

CI: initial commit #69

wants to merge 14 commits into from

Conversation

AKuHAK
Copy link

@AKuHAK AKuHAK commented Apr 2, 2022

Added precompiled binary generation based on a special Docker image.

@frno7 this step needs some preparations for it to work:

@bignaux
Copy link

bignaux commented Apr 2, 2022

I think it's more valuable for kernel devel to have non-compressed vmlinux and the modules in an archive as reference than a compressed initramfs here as artifacts. But you can still propose the two.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 3, 2022

@bignaux devs can use Docker container, actions are more meant for the final result, not for every single customization step. This action will produce the final ELF with default settings.

@frno7
Copy link
Owner

frno7 commented Apr 14, 2022

@AKuHAK, I’m considering naming that repo mipsr5900el-linux-gnu because mipsr5900el-linux-musl is a useful alternative, as described in #33 (comment). As mentioned, Musl has least two very significant advantages compared with the GNU C library.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 14, 2022

I’m considering naming that repo mipsr5900el-linux-gnu

Yes, the naming scheme is up to you.

@frno7
Copy link
Owner

frno7 commented Apr 14, 2022

@AKuHAK, we’ve now got https://github.com/frno7/mipsr5900el-linux-gnu so I suppose items 2 and 3 on your list are done?

@frno7
Copy link
Owner

frno7 commented Apr 14, 2022

@AKuHAK, an alternate name could be mipsr5900el-gentoo-linux-gnu, which would accommodate for mipsr5900el-yocto-linux-gnu- and whatever later on...

@AKuHAK
Copy link
Author

AKuHAK commented Apr 17, 2022

Great! Now it is time to change visibility in https://github.com/frno7?tab=packages

@frno7
Copy link
Owner

frno7 commented Apr 17, 2022

@AKuHAK, Git Hub indicates that the mipsr5900el-gentoo-linux-gnu package is on a MIT licence. While true for the Docker file itself, the package content is mostly (but not exclusively) GPLv2. I think we should make a directory with all licences, similar to frno7/linux/tree/ps2-main/LICENSES/.

To clarify, I propose we add the SPDX line # SPDX-License-Identifier: MIT to the top of the Docker file.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 17, 2022

Oh, licensing was always difficult for me. I just take the same license that @TobiX used.

@AKuHAK AKuHAK marked this pull request as draft April 17, 2022 18:21
@frno7
Copy link
Owner

frno7 commented Apr 17, 2022

Oh, licensing was always difficult for me. I just take the same license that @TobiX used.

No worries. :-) It occurred to me that the licence for the package scripts, and the content of the produced package, are not the same. Git Hub doesn’t seem to make much of a difference between the two, but it seems to be easy to clarify.

@frno7
Copy link
Owner

frno7 commented Apr 18, 2022

Licences are now clarified in commits frno7/gentoo-mipsr5900el@032b8bf and frno7/gentoo-mipsr5900el@f56a985.

@AKuHAK AKuHAK marked this pull request as ready for review April 19, 2022 08:07
@AKuHAK
Copy link
Author

AKuHAK commented Apr 19, 2022

This PR is ready for merging. git history is not clean, so use Squash and Merge option. If you care about future contribution to the Linux team, this file is GitHub only, so this commit should not be proposed there.

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

I could reproduce the problems observed in #28 (comment), and got it working perfectly, including USB devices, with commit frno7/gentoo-mipsr5900el@9e01f44. I was using my own kernel and IOP modules at the time.

make -j $(getconf _NPROCESSORS_ONLN) vmlinux
make -j $(getconf _NPROCESSORS_ONLN) modules
make -j $(getconf _NPROCESSORS_ONLN) modules_install
make -j $(getconf _NPROCESSORS_ONLN) vmlinuz
Copy link
Owner

Choose a reason for hiding this comment

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

Aren't these steps done in

https://github.com/frno7/mipsr5900el-gentoo-linux-gnu/blob/9e01f44b0d5e04a496c8ac27e13494bf208eb63a/Dockerfile#L49-L55

already? I agree that they're better placed here, but then I suppose we should remove them from the mipsr5900el-gentoo-linux-gnu repo? Does it make sense to make the Dockers more modular, so that QEMU would be built only in the qemu repo, and so on?

Copy link
Author

Choose a reason for hiding this comment

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

This is a big philosophical question :D Docker image currently is so unoptimized, that it is almost 3.5Gb in size and 1.7Gb reserved only for kernel sources. I made a Docker image to hold All-In-One inside one container, so everyone can download it and make a fast kernel compilation on his machine just by typing cd /srv/kernel; make vmlinuz. As about this Github Action, I agree that it is doing "double work", but I don't see that this is a problem. Docker contains Linux kernel from the past, but Github Action compiles an actively developed branch that can be more actual.
Anyway, I don't think that this action should be affected by this. You probably should create an issue in the Docker container repository for possible removing kernel sources from the container.

Does it make sense to make the Dockers more modular, so that QEMU would be built only in the qemu repo, and so on?

It can make sense, but it is difficult to calculate necessary dependencies. It is much easier to hold everything inside an all-in-one container than to manage dependency calculations for each repository. This container can be overkill for most repoes, but it is much easier to maintain. And it will be faster to complete cause all utilities are already precompiled inside the container.
Maybe only the kernel itself can be removed from container, cause it is very big and probably will not be dpendent for other things (except of initramfs of course, I mainly included it there for easier intiramfs managing).

Copy link
Owner

Choose a reason for hiding this comment

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

Right, let's celebrate this package achievement now, and tune things later as we go along. Will you push the branch name thing mentioned in #69 (comment) before we merge this? With the frno7/gentoo-mipsr5900el@9e01f44 fix in, chances are that it'll actually boot nice and proper. :-)

Copy link
Author

Choose a reason for hiding this comment

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

I am outof my pc, will be able to look at itinfew hours

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... Initialize containers fails with invalid reference format: repository name must be lowercase. It seems github.ref evaluates to refs/heads/ps2-main, which having / won’t work as a branch name. We’d need to get rid of the refs/heads/ prefix. A need for a substring function, again. :-)

Environment files seems to offer a solution. Adapting its example, something like

steps:
  - name: Obtain file tag from branch and commit
    id: obtain_file_tag
    run: |
      echo "FILE_TAG=${GITHUB_REF#refs/*/}-${GITHUB_SHA:0:8}" >> $GITHUB_ENV
  - name: Echo file tag
    id: echo_file_tag
    run: |
      echo "${{ env.FILE_TAG }}" # Should output "ps2-main-dc19f33e" or suchlike

where ${GITHUB_REF#refs/*/} (shortest matching pattern deleted) and ${GITHUB_SHA:0:8} (substring expansion) are Bash parameter expansion rules.

Copy link
Owner

Choose a reason for hiding this comment

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

According to the documentation for GITHUB_REF it’s

The branch or tag ref that triggered the workflow run. For branches this is the format refs/heads/<branch_name>, for tags it is refs/tags/<tag_name>, and for pull requests it is refs/pull/<pr_number>/merge. This variable is only set if a branch or tag is available for the event type. For example, refs/heads/feature-branch-1.

A problematic case will be pull requests with refs/pull/<pr_number>/merge having triple /. We could use ${GITHUB_REF##refs/*/} with ## serving as the longest matching pattern deleted rather than the shortest with a single #. It’ll evaluate refs/pull/17/merge to merge, rather than 17/merge, which wouldn’t work with /. Another, more useful alternative, would be to have FILE_TAG become pull-17, 17-merge, or pull-17-merge. :-)

.github/workflows/compilation.yml Outdated Show resolved Hide resolved
@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

@AKuHAK, in #28 (comment) it looks like you essentially triggered the same problems as @nickb834 did and reported in issue #66, including the error recovery NULL pointer exception in ohci-ps2.c. I suspect that depmod and modprobe in Busybox might be incompatible with their counterparts in kmod, that's used at compile time, because the unknown symbol problem vanished with

https://github.com/frno7/mipsr5900el-gentoo-linux-gnu/blob/9e01f44b0d5e04a496c8ac27e13494bf208eb63a/initramfs/ps2/sbin/init#L14

when Busybox itself regenerates the kernel module dependencies, rather than using the ones that came with the INITRAMFS, originating from make modules_install. Several modprobe commands in /sbin/init should be possible to omit, if kernel module dependencies are working properly.

#66 (comment) lists some improvement items.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 30, 2022

@frno7 what about merging this? All mentioned issues are related to the docker package (except of initramfs). If initramfs is a problem we can just build initramfsless kernel, and manage initramfs in the separate repo.

@frno7
Copy link
Owner

frno7 commented May 1, 2022

Merged, with some additional adjustments to file names and tags. Thank you for your contribution, @AKuHAK!

Let’s see if my changes work too. :-)

@frno7 frno7 force-pushed the ps2-main branch 2 times, most recently from aa044f1 to 521c48b Compare May 1, 2022 07:22
@frno7
Copy link
Owner

frno7 commented May 1, 2022

https://github.com/frno7/linux/actions/runs/2252652285 boots perfectly, with USB support, on PlayStation 2 hardware. Thanks, again. :-)

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.

3 participants