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

Provision the ability to apply the non upstream patches in any order #313

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented Mar 27, 2023

Currently, non upstream patches are applied only after upstream patches.

What i Did

  1. Apply a patch to the series files and copy the relevant patches into the patch/ folder
  2. Non upstream Patches that reside in the sonic repo will not be saved in a tar file bur rather in a folder pointed out by EXTERNAL_KERNEL_PATCH_LOC. The build variable name is also updated to INCLUDE_EXTERNAL_PATCHES
  3. INCLUDE_EXTERNAL_PATCHES will be a mandatory parameter to include NON_UPSTREAM_PATCHES

@vivekrnv
Copy link
Contributor Author

@saiarcot895, @keboliu Please review

Makefile Show resolved Hide resolved
Makefile Outdated
fi

# Precedence is given for external URL
if [ -z ${EXTERNAL_KERNEL_PATCH_URL} ] && [ x${INCLUDE_EXTERNAL_PATCHES} == xy ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we had a discussion on the variable naming in #296, but since this is being renamed to INCLUDE_EXTERNAL_PATCHES, can EXTERNAL_KERNEL_PATCH_URL fall under that, such that if INCLUDE_EXTERNAL_PATCHES is set to n, then nothing happens, even if EXTERNAL_KERNEL_PATCH_URL is specified?

Copy link
Contributor Author

@vivekrnv vivekrnv Mar 31, 2023

Choose a reason for hiding this comment

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

That means user has to provide both EXTERNAL_KERNEL_PATCH_URL & INCLUDE_EXTERNAL_PATCHES, if they wish to use non-upstream patches from an external URL. Why have two control options? INCLUDE_EXTERNAL_PATCHES is for locally saved non-up patches. atleast that's how i thought of

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saiarcot895 Do you think it makes more sense to make INCLUDE_EXTERNAL_PATCHES a mandatory parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

My thought was that because of the name INCLUDE_EXTERNAL_PATCHES, it is a parameter whether to include external patches at all, and that EXTERNAL_KERNEL_PATCH_URL is one source of those external patches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, will update it then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handled

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>
@saiarcot895 saiarcot895 merged commit 6f38dca into sonic-net:master Apr 5, 2023
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Apr 5, 2023

Request for 202211

vivekrnv added a commit to vivekrnv/sonic-buildimage that referenced this pull request Apr 5, 2023
Update sonic-linux-kernel submodule pointer to include the following:
* 6f38dca Provision the ability to apply the non upstream patches in any order ([sonic-net#313](sonic-net/sonic-linux-kernel#313))
* fbd307e Fix unable to probe emc2301/2/3 ([sonic-net#312](sonic-net/sonic-linux-kernel#312))

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>
@dgsudharsan
Copy link
Collaborator

@saiarcot895 @StormLiangMS Can you please add request for 202211 tag?

@keboliu
Copy link
Collaborator

keboliu commented Apr 8, 2023

@StormLiangMS would you please help to cherry-pick?

liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Apr 10, 2023
- Why I did it

Currently, non upstream patches are applied only after upstream patches.

Depends on sonic-net/sonic-linux-kernel#313. Can be merged in any order, preferably together

- What I did it

Non upstream Patches that reside in the sonic repo will not be saved in a tar file bur rather in a folder pointed out by EXTERNAL_KERNEL_PATCH_LOC. This is to make changes to the non upstream patches easily traceable.
The build variable name is also updated to INCLUDE_EXTERNAL_PATCHES
Files/folders expected under EXTERNAL_KERNEL_PATCH_LOC
EXTERNAL_KERNEL_PATCH_LOC/
       ├──── patches/
             ├── 0001-xxxxx.patch
             ├── 0001-yyyyyyyy.patch
             ├── .............
       ├──── series.patch
series.patch should contain a diff that is applied on the sonic-linux-kernel/patch/series file. The diff should include all the non-upstream patches.
How to verify it

Build the Kernel and verified if all the patches are applied properly

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
StormLiangMS pushed a commit that referenced this pull request Apr 13, 2023
…313)

* Provision the ability to apply the patches in any order

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

* Make INCLUDE_EXTERNAL_PATCHES a mandatory parameter

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

---------

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>
Co-authored-by: Saikrishna Arcot <sarcot@microsoft.com>
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Apr 13, 2023
- Why I did it

Currently, non upstream patches are applied only after upstream patches.

Depends on sonic-net/sonic-linux-kernel#313. Can be merged in any order, preferably together

- What I did it

Non upstream Patches that reside in the sonic repo will not be saved in a tar file bur rather in a folder pointed out by EXTERNAL_KERNEL_PATCH_LOC. This is to make changes to the non upstream patches easily traceable.
The build variable name is also updated to INCLUDE_EXTERNAL_PATCHES
Files/folders expected under EXTERNAL_KERNEL_PATCH_LOC
EXTERNAL_KERNEL_PATCH_LOC/
       ├──── patches/
             ├── 0001-xxxxx.patch
             ├── 0001-yyyyyyyy.patch
             ├── .............
       ├──── series.patch
series.patch should contain a diff that is applied on the sonic-linux-kernel/patch/series file. The diff should include all the non-upstream patches.
How to verify it

Build the Kernel and verified if all the patches are applied properly

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
vivekrnv added a commit to vivekrnv/sonic-buildimage that referenced this pull request Apr 26, 2023
- Why I did it

Currently, non upstream patches are applied only after upstream patches.

Depends on sonic-net/sonic-linux-kernel#313. Can be merged in any order, preferably together

- What I did it

Non upstream Patches that reside in the sonic repo will not be saved in a tar file bur rather in a folder pointed out by EXTERNAL_KERNEL_PATCH_LOC. This is to make changes to the non upstream patches easily traceable.
The build variable name is also updated to INCLUDE_EXTERNAL_PATCHES
Files/folders expected under EXTERNAL_KERNEL_PATCH_LOC
EXTERNAL_KERNEL_PATCH_LOC/
       ├──── patches/
             ├── 0001-xxxxx.patch
             ├── 0001-yyyyyyyy.patch
             ├── .............
       ├──── series.patch
series.patch should contain a diff that is applied on the sonic-linux-kernel/patch/series file. The diff should include all the non-upstream patches.
How to verify it

Build the Kernel and verified if all the patches are applied properly

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
vivekrnv added a commit to vivekrnv/sonic-linux-kernel that referenced this pull request May 19, 2023
…onic-net#313)

* Provision the ability to apply the patches in any order

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

* Make INCLUDE_EXTERNAL_PATCHES a mandatory parameter

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

---------

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>
Co-authored-by: Saikrishna Arcot <sarcot@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants