going Posted August 6, 2018 Posted August 6, 2018 I wish you all good health. Using the "Armbian" build system, I noticed two bad points: 1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree. 2 - Debian kernel source package includes the git directory. 1) All I do I'm fixing the version kernel (KERNELBRANCH="tag:v4.14.40") and collect several variants for one Board, changing the patchset for each version, adding or removing some. Before each new compilation I'm doing a test of the purity of the source. clean_repo() { display_alert "clean_repo $PWD" #git status -s git reset --hard HEAD # First, return the modified files. git clean -df # Then delete all new, added files. #git status } compile_kernel() { # A simple check ensures that we start with a clean slate. cd $SRC/cache/sources/$LINUXSOURCEDIR if [ "X$(git status -s)" != "X" ]; then clean_repo fi # ... ... } 2) Manufacturer's code Debian package: (--exclude='./.git/') if [[ $BUILD_KSRC != no ]]; then display_alert "Compressing sources for the linux-source package" tar cp --directory="$kerneldir" --exclude='./.git/' --owner=root . \ | pv -p -b -r -s $(du -sb "$kerneldir" --exclude=='./.git/' | cut -f1) \ | pixz -4 > $sources_pkg_dir/usr/src/linux-source-${version}-${LINUXFAMILY}.tar.xz cp COPYING $sources_pkg_dir/usr/share/doc/linux-source-${version}-${LINUXFAMILY}/LICENSE fi can be changed to view: (--exclude="[.]git") if [[ $BUILD_KSRC != no ]]; then display_alert "Compressing sources for the linux-source package" tar cp --directory="$kerneldir" --exclude="[.]git" --exclude="[.]git[a-z]*" --owner=root . \ | pv -p -b -r -s $(du -sb "$kerneldir" --exclude="[.]git" --exclude="[.]git[a-z]*" | cut -f1) \ | pixz -4 > $sources_pkg_dir/usr/src/linux-source-${version}-${LINUXFAMILY}.tar.xz cp COPYING $sources_pkg_dir/usr/share/doc/linux-source-${version}-${LINUXFAMILY}/LICENSE fi 1
chwe Posted August 6, 2018 Posted August 6, 2018 5 hours ago, going said: 1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree. got fooled by this too in the beginnings.. I mostly work with 'CLEAN_LEVEL = sources' when I prepare patches which create new files (https://github.com/armbian/build/blob/d9849de58fcb8fe8f6c0b01d71b61c28a6bc6f35/config/templates/config-example.conf#L6-L10). your solution looks smart (but to be honest, my git-fu is to low to be sure that this works reliable). You may create a PR where the maintainers will review it this works sufficient? Would save some bandwidth when preparing such patches cause I normally need more than one attempt for a proper patch.
going Posted August 6, 2018 Author Posted August 6, 2018 22 minutes ago, chwe said: 'CLEAN_LEVEL = sources' large overhead when downloading. git reset --hard HEAD # First, return the modified files. git clean -df # Then delete all new, added files. This code is guaranteed to work correctly. I use it for a month. "Clean slate" before you start the compilation, and no issues with the new patches.
chwe Posted August 6, 2018 Posted August 6, 2018 3 minutes ago, going said: This code is guaranteed to work correctly. I use it for a month. "Clean slate" before you start the compilation, and no issues with the new patches. Well, then I'm happy to see a PR from it and others probably too.. overhead isn't that much an issue here.. thanks to fiber at home.. but for sure, if it works reliable I would still use it.
going Posted August 6, 2018 Author Posted August 6, 2018 11 minutes ago, chwe said: overhead isn't that much an issue here.. thanks to fiber at home.. I am somewhere in the Siberian province and use 3G modem.
chwe Posted August 6, 2018 Posted August 6, 2018 that's why I like your approach. Problem is, as long as downloading big junks of code doesn't hurt you, you don't care much. You may team up with @Igor or @zador.blood.stained how this can be implemented properly. I guess it should be here: https://github.com/armbian/build/blob/88ea83e694d8c015c42a9ae6fb163b0e7470755e/lib/general.sh#L32-L81 But, to be honest.. I'm not 100% familiar with this part of the build-script to guide you how this should be done properly. I'm sure, they are. 17 minutes ago, going said: I am somewhere in the Siberian province and use 3G modem. That's cool. Seeing that the buildscript is used around the world and limited bandwidth helps that people think about issues which aren't 'that much an issue' for those without such limitations. I really hope we can implement it so that it gets default (or at least everyone can set it in the configs) for all armbian users.
going Posted August 6, 2018 Author Posted August 6, 2018 I noticed this problem after I stopped the kernel version from moving up on a particular tag and started to change patches to remove errors. Turned off a few patches to clear the field for observation. And saw that the big aufs patch (it adds a lot of new files) partially applied, causing even more errors, but my fixes have no effect.
xenpac Posted September 15, 2018 Posted September 15, 2018 On 8/6/2018 at 5:44 PM, going said: I am somewhere in the Siberian province and use 3G modem. Hi Going, Siberian province sounds nice ! ;-)
going Posted November 23, 2018 Author Posted November 23, 2018 On 8/6/2018 at 11:03 AM, going said: 1 - incomplete cleaning of the source code before starting a new compilation, seen when patches add new files to the tree. After many inspections, it is possible to fix this problem. Spoiler Fix incomplete cleaning of the source code. Removed duplicate code and the unit is not correctly coupled logic. --- lib/general.sh | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/lib/general.sh b/lib/general.sh index 8de1a352..57ee4e64 100644 --- a/lib/general.sh +++ b/lib/general.sh @@ -223,52 +223,42 @@ fetch_from_repo() git remote add origin $url fi - local changed=false - local local_hash=$(git rev-parse @ 2>/dev/null) case $ref_type in branch) # TODO: grep refs/heads/$name local remote_hash=$(git ls-remote -h $url "$ref_name" | head -1 | cut -f1) - [[ -z $local_hash || $local_hash != $remote_hash ]] && changed=true + [[ -z $local_hash || $local_hash != $remote_hash ]] && \ + git fetch --depth 1 origin $ref_name ;; tag) local remote_hash=$(git ls-remote -t $url "$ref_name" | cut -f1) if [[ -z $local_hash || $local_hash != $remote_hash ]]; then remote_hash=$(git ls-remote -t $url "$ref_name^{}" | cut -f1) - [[ -z $remote_hash || $local_hash != $remote_hash ]] && changed=true + [[ -z $remote_hash || $local_hash != $remote_hash ]] && \ + git fetch --depth 1 origin tags/$ref_name fi ;; head) local remote_hash=$(git ls-remote $url HEAD | cut -f1) - [[ -z $local_hash || $local_hash != $remote_hash ]] && changed=true + [[ -z $local_hash || $local_hash != $remote_hash ]] && \ + git fetch --depth 1 origin HEAD ;; esac - if [[ $changed == true ]]; then - # remote was updated, fetch and check out updates - display_alert "Fetching updates" - case $ref_type in - branch) git fetch --depth 1 origin $ref_name ;; - tag) git fetch --depth 1 origin tags/$ref_name ;; - head) git fetch --depth 1 origin HEAD ;; - esac - display_alert "Checking out" - git checkout -f -q FETCH_HEAD - elif [[ -n $(git status -uno --porcelain --ignore-submodules=all) ]]; then - # working directory is not clean - if [[ $FORCE_CHECKOUT == yes ]]; then - display_alert "Checking out" - git checkout -f -q HEAD - else - display_alert "Skipping checkout" - fi - else - # working directory is clean, nothing to do - display_alert "Up to date" + # Return the files that are tracked by git to the initial state. + display_alert "Checking out" + git checkout -f -q FETCH_HEAD + if [ "blank$(git status -s)" != "blank" ]; then + # Files that are not tracked by git and were added + # when the patch was applied must be removed. + display_alert "Cleaning source" "$PWD" + git clean -df fi + display_alert "Up to date" + if [[ -f .gitmodules ]]; then display_alert "Updating submodules" "" "ext" # FML: http://stackoverflow.com/a/17692710 -- 2.13.7 0001-Fix-incomplete-cleaning-of-the-source-code.patch
going Posted November 23, 2018 Author Posted November 23, 2018 Spoiler Check that to understand what it is talking. Optional patch. Install at the beginning to see the files added during the previous patching and compilation. --- lib/main.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/main.sh b/lib/main.sh index e2e274a1..cdc50156 100644 --- a/lib/main.sh +++ b/lib/main.sh @@ -275,6 +275,15 @@ for option in $(tr ',' ' ' <<< "$CLEAN_LEVEL"); do [[ $option != sources ]] && cleaning "$option" done +# Checking for purity src atf, uboot, kernel +# -it's just information for thought. +cur_dir=$PWD +LIST_SRC="$SRC/cache/sources/$ATFSOURCEDIR $SRC/cache/sources/$BOOTSOURCEDIR $SRC/cache/sources/$LINUXSOURCEDIR" +for IN in $LIST_SRC; do + cd $IN; display_alert "In source of $IN dirty files: " "$(git status -s | wc -l)" +done +cd $cur_dir + # Compile u-boot if packed .deb does not exist if [[ ! -f $DEST/debs/${CHOSEN_UBOOT}_${REVISION}_${ARCH}.deb ]]; then if [[ -n $ATFSOURCE ]]; then -- 2.13.7 0002-Check-that-to-understand-what-it-is-talking.patch
Igor Posted November 26, 2018 Posted November 26, 2018 I looks OK ... let's test a bit but I guess you can sent a PR to the build script. Thanks!
guidol Posted November 26, 2018 Posted November 26, 2018 On 8/6/2018 at 6:44 PM, going said: I am somewhere in the Siberian province and use 3G modem. Ohh - then are my 8MBit here in turkey are better. But when I could get a LTE-Flat it would be faster than the 8MBit
going Posted November 26, 2018 Author Posted November 26, 2018 5 hours ago, Igor said: I looks OK ... let's test a bit but I guess you can sent a PR to the build script. Thanks! Igor, sorry but the automatic translation does not allow me to understand, you must also send the patches somewhere else? Or is it just enough to put them here? I did not find a description of the patch acceptance procedure in the documentation. I have a lot of fixes. Well tested I want to send for consideration to acceptance.
going Posted November 26, 2018 Author Posted November 26, 2018 2 hours ago, guidol said: I am somewhere in the Siberian province and use 3G modem. In fact, I'm good with the Internet and all the necessary repositories are on a nearby SSD. There can be several reasons: - Slow Internet and unwillingness to clean sources every time. - Inattention, forgetfulness, typo. - Ignorance of documentation. In any negative scenario, the source directories must be clean before compiling.
Igor Posted November 26, 2018 Posted November 26, 2018 1 hour ago, going said: you must also send the patches somewhere else? I can add patches but since they are not signed they will retain my name. I want to avoid that whenever possible, especially when author is known. The right way is to create a pull request. 1 hour ago, going said: I have a lot of fixes. Than is great to hear! Then it is even more important to learn how to do it properly. Here are some general instructions how to create a pull request on Github: https://www.armbian.com/get-involved/#submit https://docs.armbian.com/Process_Contribute I am pretty sure there must be some guides in your native language if Google translate fails.
going Posted November 27, 2018 Author Posted November 27, 2018 13 hours ago, Igor said: Than is great to hear! Then it is even more important to learn how to do it properly. Thanks. I saw the applied patches for testing. I read your links and realized that it is customary to make changes in a separate branch and then pull request. Of course, first we need to discuss with the community the need to develop new functionality. I will do so in the future. I'm currently laying out a number of patches that I think are the most important. Although it is not a tradition. You can apply them on your own behalf, after testing, if you see them reasonable. I don't claim authorship. Thank you for your understanding. 1
going Posted March 14, 2019 Author Posted March 14, 2019 Igor, let's try to open the eyes of users. Please apply these two patches. 0001-Correction-stderror-to-file.patch 0002-Install-to-see-the-status.patch
lanefu Posted March 14, 2019 Posted March 14, 2019 7 hours ago, going said: Igor, let's try to open the eyes of users. Please apply these two patches. Hey thanks for putting in a lot of effort to make the patches. Could you submit them as a PR on https://github.com/armbian/build There are a few others besides igor that can apply discretion and merge practical fixes.
going Posted March 17, 2019 Author Posted March 17, 2019 On 3/14/2019 at 7:29 PM, lanefu said: Could you submit them as a PR on https://github.com/armbian/build Ok. The first one is ready.
lanefu Posted March 17, 2019 Posted March 17, 2019 5 hours ago, going said: Ok. The first one is ready. merged https://github.com/armbian/build/pull/1314 thanks a bunch... i'll keep an eye out for your next PR
going Posted March 17, 2019 Author Posted March 17, 2019 I apologize if I seem rude. I use automatic translation. Sometimes the meaning can turn over. And thank you. I started working with github. Now I understand why you don't accept the patches as a patch file 1
Recommended Posts