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

build: fixup 9534357 typo #3520

Closed
wants to merge 1 commit into from
Closed

build: fixup 9534357 typo #3520

wants to merge 1 commit into from

Conversation

aparcar
Copy link
Member

@aparcar aparcar commented Oct 18, 2020

Fixup 9534357 which added a non existing packages to the packages-y
list.

Reported-by: Jonas Albrecht plonkbong100@protonmail.com
Reported-by: Rosen Penev rosenp@gmail.com
Signed-off-by: Paul Spooren mail@aparcar.org

@aparcar
Copy link
Member Author

aparcar commented Oct 18, 2020

@nbd168 ping

@aparcar
Copy link
Member Author

aparcar commented Oct 18, 2020

@blogic ping

@adschm
Copy link
Member

adschm commented Oct 18, 2020

Can you please change the commit title so it tells what you are changing?

@adschm adschm added build/scripts/tools pull request/issues for build, scripts and tools related changes needs changes labels Oct 18, 2020
@adschm
Copy link
Member

adschm commented Oct 18, 2020

And please add a proper:

Fixes: 12digitcommit id ("commit title")

to the commit message.

Fixes: 9534357 build: always build package/kernel/linux

Reported-by: Jonas Albrecht <plonkbong100@protonmail.com>
Reported-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Paul Spooren <mail@aparcar.org>
@neheb
Copy link
Contributor

neheb commented Oct 18, 2020

ACK. This is breaking the buildbot in the packages feed. Almost all the red arrows are a result of this: https://github.com/openwrt/packages/pulls

@nbd168
Copy link
Member

nbd168 commented Oct 19, 2020

Having 'kernel/linux' in there is not a typo. The main problem is the fact that the SDK and the main build system have the 'linux' package in different subdirectories. I'll fix that part instead, so we don't need this PR.

@nbd168
Copy link
Member

nbd168 commented Oct 19, 2020

Please try this commit: https://git.openwrt.org/?p=openwrt/staging/nbd.git;a=commitdiff;h=921d4500825f837335f998656f0bb88ef0920d55

@aparcar
Copy link
Member Author

aparcar commented Oct 19, 2020

Tested and works, I'd suggest to create the kernel folder like shown below to save a mkdir call.

diff --git a/target/sdk/Makefile b/target/sdk/Makefile
index 022a791ebf..67061861c0 100644
--- a/target/sdk/Makefile
+++ b/target/sdk/Makefile
@@ -82,7 +82,7 @@ KERNEL_FILES := $(patsubst $(TOPDIR)/%,%,$(wildcard $(addprefix $(LINUX_DIR)/,$(
 all: compile
 
 $(BIN_DIR)/$(SDK_NAME).tar.xz: clean
-       mkdir -p $(SDK_BUILD_DIR)/dl $(SDK_BUILD_DIR)/package
+       mkdir -p $(SDK_BUILD_DIR)/dl $(SDK_BUILD_DIR)/package/kernel
        $(CP) -L $(INCLUDE_DIR) $(SCRIPT_DIR) $(SDK_BUILD_DIR)/
        $(TAR) -cf - -C $(TOPDIR) \
                `cd $(TOPDIR); find $(KDIR_BASE)/ -name \*.ko` \
@@ -132,8 +132,10 @@ $(BIN_DIR)/$(SDK_NAME).tar.xz: clean
        $(CP) \
                $(TOPDIR)/package/Makefile \
                $(TOPDIR)/package/libs/toolchain \
-               $(TOPDIR)/package/kernel/linux \
                $(SDK_BUILD_DIR)/package/
+       $(CP) \
+               $(TOPDIR)/package/kernel/linux \
+               $(SDK_BUILD_DIR)/package/kernel
 
        -rm -rf $(SDK_BUILD_DIR)/$(STAGING_SUBDIR_HOST)/.prereq-build
 

@aparcar aparcar deleted the itslinux branch October 19, 2020 20:06
@predators46
Copy link

no changes in the master branch

@aparcar
Copy link
Member Author

aparcar commented Oct 21, 2020

@nbd168 ping

@mayli mayli mentioned this pull request Oct 25, 2020
@BKPepe
Copy link
Member

BKPepe commented Oct 25, 2020

@adschm @ynezz This is unbelievable. Is anybody willing to do some changes to the master branch?
Package maintainers in packages repository are merging bumps, bug fixes even though almost all checks are failing there!

What we should do with this? We are now sitting here and waiting to fix the culprit for at least a week here!

Ping: @nbd168

@adschm
Copy link
Member

adschm commented Oct 25, 2020

@BKPepe This touches an area I have almost no experience with. Broken or not, I certainly won't mess with something I don't understand.

@BKPepe
Copy link
Member

BKPepe commented Oct 26, 2020

So rather leaving it as broken and looking bad (failed checks) instead of reverting it. Did I understand it correctly or not? Anyway, @blocktrron cherry-picked it.

@predators46
Copy link

So rather leaving it as broken and looking bad (failed checks) instead of reverting it. Did I understand it correctly or not? Anyway, @blocktrron cherry-picked it.

the problem is here
package-y + = kernel/linux

@adschm
Copy link
Member

adschm commented Oct 26, 2020

So rather leaving it as broken and looking bad (failed checks) instead of reverting it. Did I understand it correctly or not?

Yes, it's not my job to clean up behind others. Experience tells that people messing with or trying to fix stuff they don't understand don't make it better anyway.
Apart from that, this was opened and closed by aparcar, so it actually did look like someone is taking care anyway.

@aparcar
Copy link
Member Author

aparcar commented Oct 26, 2020

Apart from that, this was opened and closed by aparcar, so it actually did look like someone is taking care anyway.

Yes I close PRs here once they are pulled into staging trees. I didn't except nothing would follow after the cherry-pick. Anyway, I'll leave future PRs open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/scripts/tools pull request/issues for build, scripts and tools related changes needs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants