Make uSD Card Image for the Audio Mini #24

dack-fe · 2021-01-07T23:57:40Z

Merging the build scripts, Dockerfile, and Jenkinsfile into dev for the Audio Mini Linux Image Builder

…ke_sdcard_dev

…cript for making image for audiomini

…tils into make_sdcard_dev

Removed the curl install Changed the starting script to the audiomini build script

fe-wickham

Looks good overall but it'd be nice to see the couple changes I mention in my other comments before merge

image_builder/audiomini/docker_build_audiomini.sh

image_builder/audiomini/Dockerfile

image_builder/README.md

tvannoy

Without being able to test it, it looks good to me. Glad to see the tool by robseb being used.

tvannoy · 2021-01-08T18:32:20Z

image_builder/audiomini/build_audiomini_image.sh

+else
+    if [ -f frost_rootfs.tar.gz ]; then
+        echo 'Using the local Root File System.'
+        tar -xhzvf frost_rootfs.tar.gz; 


Does this preserve symbolic links? I guess if the final image works, then it doesn't matter.

I don't believe it does given the -h is described as follow symlinks; archive and dump the files they point to (it does the same for hardlinks as well).

I feel like presently we are using the -h out of necessity because I recall problems when extracting without that flag but I would have to investigate further to know what problems we are running into without it.

Ah, I see. So links are just being replaced by the files.

From my recollection, things will be messed up, and probably won't boot, if links are ignored. I recall running into issues where things like /bin/sh couldn't be found, leading to kernel panics.

I believe -p preserves symbolic links, which would be an alternative to -h.

fe-tdavis

I tested the full process and the image built and ran successfully although

image_builder/audiomini/ReadMe.md

fe-tdavis · 2021-01-08T19:26:24Z

Without being able to test it

I'm going to be a bit of a stickler here... If you are going to review our pull requests, please don't approve something you can't test.

tvannoy · 2021-01-08T19:56:46Z

I'm going to be a bit of a stickler here... If you are going to review our pull requests, please don't approve something you can't test.

@fe-tdavis Fair enough. I will only make comments without explicit approval if I am not able to test. I think it's a good policy to make sure tests pass before merging. I am happy to keep reviewing PRs, especially when my review is requested.

Can’t test

fe-tdavis · 2021-01-08T20:15:30Z

@fe-tdavis Fair enough. I will only make comments without explicit approval if I am not able to test. I think it's a good policy to make sure tests pass before merging. I am happy to keep reviewing PRs, especially when my review is requested.

👍

dack-fe · 2021-01-08T20:31:30Z

What is the status on the symbolic links? Have they been tested? Is it an issue?

fe-tdavis · 2021-01-08T21:10:19Z

What is the status on the symbolic links? Have they been tested? Is it an issue?

I did a quick test with the system as is and it appears to work. I've used the -h flag in the past with my root filesystems and I haven't run into anything odd that I can remember... I don't have a preference if we using the -h or switch to -p if that works.

fe-tdavis

Works as is, but if we change to the -p flag we should do further testing

tvannoy · 2021-01-08T21:56:39Z

I misspoke earlier. -p is for preserve permissions, which is default when run as superuser (we do want permissions preserved).

I think it is preferable to keep the symlinks rather than dereferencing them, but I don't know if dereferencing causes any practical problems. Judging by reports so far, it doesn't appear to be an issue. My thought is that if symlinks are there in the original Ubuntu base archive, there is probably a reason, so we should keep the symlinks.

I would think using the same tar arguments that we use in build_rootfs.sh would work correctly, though, as I don't recall having any issues with that before.

We can verify if we are removing symlinks by running find with -type l on the original Ubuntu base directory and our modified rootfs.

fe-wickham · 2021-01-08T22:07:18Z

I think it is preferable to keep the symlinks rather than dereferencing them, but I don't know if dereferencing causes any practical problems. Judging by reports so far, it doesn't appear to be an issue. My thought is that if symlinks are there in the original Ubuntu base archive, there is probably a reason, so we should keep the symlinks.

I am confident it is preferable to preserve symlinks, hence I have created #25 but I don't believe it is worth stopping this from being merged because there isn't much reason for symlinks to break the image as built from working. What I would be concerned about is future configuration and maintenance of a running system because something may be setup currently with symlinks such that it only updates 1 file and without the symlinks it won't update the other files.

fe-wickham

With the minor issues I raised addressed, this all looks great now

tvannoy · 2021-01-08T22:13:29Z

I think it is preferable to keep the symlinks rather than dereferencing them, but I don't know if dereferencing causes any practical problems. Judging by reports so far, it doesn't appear to be an issue. My thought is that if symlinks are there in the original Ubuntu base archive, there is probably a reason, so we should keep the symlinks.

I am confident it is preferable to preserve symlinks, hence I have created #25 but I don't believe it is worth stopping this from being merged because there isn't much reason for symlinks to break the image as built from working. What I would be concerned about is future configuration and maintenance of a running system because something may be setup currently with symlinks such that it only updates 1 file and without the symlinks it won't update the other files.

Yes, exactly. I only see it being an issue if (most likely when) a package update updates a file (library or executable, most likely) that is supposed to be the target of a symlink. Symlinks are really common with .so libraries.

I agree it isn't worth holding this PR up, as the image does work as is. Fully testing and getting symlinks preserved will take some while because the rootfs building/flashing operations are quite slow.

fe-tdavis and others added 30 commits October 5, 2020 13:33

Added initial files

db752d8

Rough draft of the instructions

2410010

Merge branch 'dev' of github.com:fpga-open-speech-tools/utils into ma…

38c1200

…ke_sdcard_dev

Updated sdimage script with working directory option and added bash s…

9d307c8

…cript for making image for audiomini

Merge branch 'make_sdcard_dev' of github.com:fpga-open-speech-tools/u…

df636b1

…tils into make_sdcard_dev

Added checks to download the Image artifacts from S3

a267e07

Added blueprint for creating uSD images

6407f6f

Documentation update

0fd9ae9

Updated the build script

69a9475

Moved the Build Audio Mini Image Script

3df87d6

Initial Commit

f1e9603

Updated the Dockerfile for the audiomini build

c9114a3

Removed the curl install Changed the starting script to the audiomini build script

Updated the file name

15af6d6

Updated the docker_build script for the Audio Mini Linux Image

e70f659

Updated the docker build script name

b6cbc39

Removed the folder/file checks

94cdf72

Updated the gitignore

9ffd5b1

Added Giti and Python3

85b9f06

Updated the checkout

7f62cfe

Working on Debugging the Docker Build

d2c971b

Debugging with Dylan

e79e29b

Updated the folder name

2a6994c

Initial Commit

7b8c12c

Copying Artifacts from Jenkins

edf1aff

Merge branch 'dev' into make_sdcard_dev

5054c4c

Merge branch 'dev' into make_sdcard_dev

5c99ec4

Added the Image Builder Folder Structure

62632b1

Updated the Jenkinsfile to check the image builder folder

7245d7c

Removed the Procedure Section

ed74a31

Removed the old Altera Python Script

80ff667

fe-wickham reviewed Jan 8, 2021

View reviewed changes

image_builder/audiomini/docker_build_audiomini.sh Outdated Show resolved Hide resolved

image_builder/audiomini/Dockerfile Outdated Show resolved Hide resolved

image_builder/README.md Show resolved Hide resolved

dack-fe added 3 commits January 8, 2021 09:29

Removed the debugging line that was commetted out

6868616

Updated the git ignore with the folder change

8f91a9e

Added comments to the Docker file

51025f7

dack-fe requested review from fe-wickham and tvannoy January 8, 2021 16:44

dack-fe added 2 commits January 8, 2021 11:04

Added copy artifact of the Audio Mini Passthrough RBF

7e0080c

Update the project name

abce0d0

tvannoy previously approved these changes Jan 8, 2021

View reviewed changes

Updated Jenkins Project Name

2b6b53d

fe-tdavis reviewed Jan 8, 2021

View reviewed changes

image_builder/audiomini/ReadMe.md Show resolved Hide resolved

Updated back to the Linux LKM Jenkins Pipeline

78ac910

tvannoy self-requested a review January 8, 2021 20:00

fe-tdavis closed this Jan 8, 2021

fe-tdavis reopened this Jan 8, 2021

fe-tdavis approved these changes Jan 8, 2021

View reviewed changes

fe-wickham mentioned this pull request Jan 8, 2021

Investigate extracting the rootfs with symlinks preserved #25

Open

fe-wickham approved these changes Jan 8, 2021

View reviewed changes

dack-fe merged commit 717ec49 into dev Jan 8, 2021

dack-fe deleted the make_sdcard_dev branch January 8, 2021 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make uSD Card Image for the Audio Mini #24

Make uSD Card Image for the Audio Mini #24

dack-fe commented Jan 7, 2021

fe-wickham left a comment

tvannoy left a comment

tvannoy Jan 8, 2021

fe-wickham Jan 8, 2021 •

edited

Loading

tvannoy Jan 8, 2021

fe-tdavis left a comment

fe-tdavis commented Jan 8, 2021

tvannoy commented Jan 8, 2021

fe-tdavis commented Jan 8, 2021

dack-fe commented Jan 8, 2021

fe-tdavis commented Jan 8, 2021

fe-tdavis left a comment

tvannoy commented Jan 8, 2021

fe-wickham commented Jan 8, 2021

fe-wickham left a comment

tvannoy commented Jan 8, 2021

Make uSD Card Image for the Audio Mini #24

Make uSD Card Image for the Audio Mini #24

Conversation

dack-fe commented Jan 7, 2021

fe-wickham left a comment

Choose a reason for hiding this comment

tvannoy left a comment

Choose a reason for hiding this comment

tvannoy Jan 8, 2021

Choose a reason for hiding this comment

fe-wickham Jan 8, 2021 • edited Loading

Choose a reason for hiding this comment

tvannoy Jan 8, 2021

Choose a reason for hiding this comment

fe-tdavis left a comment

Choose a reason for hiding this comment

fe-tdavis commented Jan 8, 2021

tvannoy commented Jan 8, 2021

fe-tdavis commented Jan 8, 2021

dack-fe commented Jan 8, 2021

fe-tdavis commented Jan 8, 2021

fe-tdavis left a comment

Choose a reason for hiding this comment

tvannoy commented Jan 8, 2021

fe-wickham commented Jan 8, 2021

fe-wickham left a comment

Choose a reason for hiding this comment

tvannoy commented Jan 8, 2021

fe-wickham Jan 8, 2021 •

edited

Loading