Fix sonic frr makefile to handle multiple builds #3483

nikos-github · 2019-09-19T06:14:32Z

This fixes #3407.

The logic is to try a dry run on the patch and if that passes, apply the patch. If not, try a dry run removal. If it passes, the patch is already applied and the code hasn't changed significantly to affect it. If it doesn't pass, fail the build as the patch most probably needs to be reworked.

@lguohan @pavel-shirshov Please review.

FYI: @ben-gale @MichelMoriniaux @mslocrian @heidinet2007

Sync to master

pavel-shirshov · 2019-09-24T18:22:51Z

Hi Nikos,

Thank you for the patch. Can we make a loop over an array?
for patch_file in patch_file1 patch_file2 patch_file3
do

done
Also we could make a list of patches from ls of the patches directory.

ben-gale · 2019-09-24T21:30:47Z

+Dilip

…

On Tue, Sep 24, 2019 at 2:22 PM pavel-shirshov ***@***.***> wrote: Hi Nikos, Thank you for the patch. Can we make a loop over an array? for patch_file in patch_file1 patch_file2 patch_file3 do done Also we could make a list of patches from ls of the patches directory. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3483>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPRCWZVHFMX2ZMY3KXIQPDQLJLIFANCNFSM4IYG3LDQ> .

lguohan · 2019-09-24T22:17:04Z

#3480 is a much simpler way to fix this issue. but I do not know why stg init fails?

nikos-github · 2019-09-24T22:19:33Z

Hi Nikos,

Thank you for the patch. Can we make a loop over an array?
for patch_file in patch_file1 patch_file2 patch_file3
do

done

Will do.

Also we could make a list of patches from ls of the patches directory.

I'm really not keen on parsing screen output in the makefile. How strongly do you feel about this?

nikos-github · 2019-09-24T22:21:41Z

#3480 is a much simpler way to fix this issue. but I do not know why stg init fails?

@lguohan I don't think that's the right approach. Issuing git reset --hard will cause any user changes to be lost.

sudhanshukumar22 · 2019-09-25T04:36:59Z

src/sonic-frr/Makefile

+			exit 1
+		fi
+	fi
+	if patch -sfp1 --dry-run < ../patch/$(patch5); then


Hi,
what does "if patch -sfp1 --dry-run < ../patch/$(patch5);" mean ? Also, I am adding a new patch6. For every new patch, should we change the sonic-frr/makefile.

Yes, with each patch the makefile would have to be changed. if patch -sfp1 --dry-run < ../patch/$(patch5) tries (without applying hence the dry-run) to see if the patch can apply.

nikos-github · 2019-09-25T16:25:06Z

retest this please

nikos-github · 2019-09-25T18:13:32Z

retest this please

sudhanshukumar22 · 2019-09-25T18:25:40Z

src/sonic-frr/Makefile

@@ -10,7 +10,7 @@ patch2 = 0002-Reduce-severity-of-Vty-connected-from-message.patch
 patch3 = 0003-ignore-nexthop-attribute-when-NLRI-is-present.patch
 patch4 = 0004-Allow-BGP-attr-NEXT_HOP-to-be-0.0.0.0-due-to-allevia.patch
 patch5 = 0005-Support-VRF.patch
-patches = "$patch1 $patch2 $patch3 $patch4 $patch5"
+patches = "$(patch1) $(patch2) $(patch3) $(patch4) $(patch5)"


Hi Nikos,
I needed this patch for my broadcom image. I had taken your changes and made some additions since I needed it urgently for submitting my changes. The below script works for broadcom image.
.ONESHELL:
SHELL = /bin/bash
.SHELLFLAGS += -e

MAIN_TARGET = $(FRR)
DERIVED_TARGET = $(FRR_PYTHONTOOLS) $(FRR_DBG) $(FRR_SNMP) $(FRR_SNMP_DBG)

patch1 = 0001-Add-support-of-bgp-tcp-DSCP-value.patch
patch2 = 0002-Reduce-severity-of-Vty-connected-from-message.patch
patch3 = 0003-ignore-nexthop-attribute-when-NLRI-is-present.patch
patch4 = 0004-Allow-BGP-attr-NEXT_HOP-to-be-0.0.0.0-due-to-allevia.patch
patch5 = 0005-Support-VRF.patch

$(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
# Build the package
pushd ./frr
for frr_patch in $(patch1) $(patch2) $(patch3) $(patch4) $(patch5); do
echo "Patch $$frr_patch"
if patch -sfp1 --dry-run < ../patch/$$frr_patch; then
patch -sfp1 < ../patch/$$frr_patch
else
if ! patch -Rsfp1 --dry-run < ../patch/$$frr_patch; then
echo "Patch $$frr_patch failure"
exit 1
fi
fi
done
tools/tarsource.sh -V -e '-sonic'
dpkg-buildpackage -rfakeroot -b -us -uc -Ppkg.frr.nortrlib -j$(SONIC_CONFIG_MAKE_JOBS)
popd
mv $(DERIVED_TARGET) $* $(DEST)/

$(addprefix $(DEST)/, $(DERIVED_TARGET)): $(DEST)/% : $(DEST)/$(MAIN_TARGET)

Yes my latest patch is somewhat similar.

You need to replace $(frr_patch) by $$frr_patch. This is because, for a loop variable, we should not let the makefile change the frr_patch content.

Thank you for the correction.

pavel-shirshov

Good for now

nikos-github · 2019-09-27T22:22:36Z

@lguohan Can we merge this?

pavel-shirshov · 2019-09-28T01:48:42Z

@nikos-github Currently I'm working on #3480
Which makes the same but with stg. Guohan asked me to use stg for applying patches into sonic-frr

nikos-github · 2019-09-28T02:29:19Z

@nikos-github Currently I'm working on #3480
Which makes the same but with stg. Guohan asked me to use stg for applying patches into sonic-frr

@pavel-shirshov Is your solution considering private changes? Can you explain how it will work if someone has modified frr code and wants to re-build?

The solution in this PR will work with private changes too if any of the patches are not affected.

pavel-shirshov · 2019-10-03T20:39:59Z

Close in favor of #3480

…atically (#20122) #### Why I did it src/sonic-utilities ``` * d29b8241 - (HEAD -> master, origin/master, origin/HEAD) Revert "[wol] Implement wol command line utility" (#3515) (3 hours ago) [Wenda Chu] * 9357c45f - [chassis][cli] Fix config chassis module startup/shutdown command for fabric module (#3483) (9 hours ago) [Marty Y. Lok] * 4c7e54a9 - [qos reload] Fix "config qos reload" overriding entire CONFIG_DB (#3479) (18 hours ago) [Stepan Blyshchak] * 544584ea - Add back the option f to the reboot script (#3492) (18 hours ago) [DavidZagury] * dc955e80 - [Mellanox] Add CMIS Host Management Files to 'show techsupport' Dumps (#3501) (18 hours ago) [Tomer Shalvi] ``` #### How I did it #### How to verify it #### Description for the changelog

…atically (#20188) #### Why I did it src/sonic-utilities ``` * 3a0575c2 - (HEAD -> 202405, origin/202405) Add back the option f to the reboot script (#3492) (4 hours ago) [DavidZagury] * 60c14df8 - Enable show interfacess counters on chassis supervisor (#3488) (4 hours ago) [Changrong Wu] * cbbfe7b7 - [chassis][cli] Fix config chassis module startup/shutdown command for fabric module (#3483) (4 hours ago) [Marty Y. Lok] * b6cbe6ed - Remove redundant mmuconfig file (#3446) (4 hours ago) [HP] * fb6dd589 - [qos reload] Fix "config qos reload" overriding entire CONFIG_DB (#3479) (4 hours ago) [Stepan Blyshchak] * 9da5db58 - Skip default lanes dup check (#3489) (4 hours ago) [Xincun Li] * edde02ce - [Mellanox] Add CMIS Host Management Files to 'show techsupport' Dumps (#3501) (4 hours ago) [Tomer Shalvi] ``` #### How I did it #### How to verify it #### Description for the changelog

…atically (sonic-net#20122) #### Why I did it src/sonic-utilities ``` * d29b8241 - (HEAD -> master, origin/master, origin/HEAD) Revert "[wol] Implement wol command line utility" (sonic-net#3515) (3 hours ago) [Wenda Chu] * 9357c45f - [chassis][cli] Fix config chassis module startup/shutdown command for fabric module (sonic-net#3483) (9 hours ago) [Marty Y. Lok] * 4c7e54a9 - [qos reload] Fix "config qos reload" overriding entire CONFIG_DB (sonic-net#3479) (18 hours ago) [Stepan Blyshchak] * 544584ea - Add back the option f to the reboot script (sonic-net#3492) (18 hours ago) [DavidZagury] * dc955e80 - [Mellanox] Add CMIS Host Management Files to 'show techsupport' Dumps (sonic-net#3501) (18 hours ago) [Tomer Shalvi] ``` #### How I did it #### How to verify it #### Description for the changelog

nikos-github added 2 commits April 23, 2019 18:51

Merge pull request #11 from Azure/master

7bffe35

Sync to master

Merge pull request #12 from Azure/master

bd77348

Sync to master

jleveque added the Enhancement ➕ label Sep 19, 2019

pavel-shirshov self-requested a review September 24, 2019 18:20

pavel-shirshov self-assigned this Sep 24, 2019

sudhanshukumar22 reviewed Sep 25, 2019

View reviewed changes

Fix sonic frr makefile to handle multiple builds

e20c213

nikos-github force-pushed the frr-makefile branch from f5d83f1 to e20c213 Compare September 25, 2019 21:17

pavel-shirshov approved these changes Sep 25, 2019

View reviewed changes

nikos added 4 commits September 25, 2019 15:13

Fix sonic frr makefile to handle multiple builds

74b49f9

Fix sonic frr makefile to handle multiple builds

8b5d818

Fix sonic frr makefile to handle multiple builds

0e0fc74

Fix sonic frr makefile to handle multiple builds

0f650cd

nikos-github mentioned this pull request Sep 29, 2019

[FRR]: Use stg instead of patch #3480

Merged

Merge branch 'master' into frr-makefile

e477b15

pavel-shirshov closed this Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sonic frr makefile to handle multiple builds #3483

Fix sonic frr makefile to handle multiple builds #3483

nikos-github commented Sep 19, 2019 •

edited

Loading

pavel-shirshov commented Sep 24, 2019

ben-gale commented Sep 24, 2019 via email

lguohan commented Sep 24, 2019

nikos-github commented Sep 24, 2019

nikos-github commented Sep 24, 2019

sudhanshukumar22 Sep 25, 2019

nikos-github Sep 25, 2019

nikos-github commented Sep 25, 2019

nikos-github commented Sep 25, 2019

sudhanshukumar22 Sep 25, 2019

nikos-github Sep 25, 2019

sudhanshukumar22 Sep 25, 2019

nikos-github Sep 26, 2019

pavel-shirshov left a comment

nikos-github commented Sep 27, 2019

pavel-shirshov commented Sep 28, 2019

nikos-github commented Sep 28, 2019 •

edited

Loading

pavel-shirshov commented Oct 3, 2019

Fix sonic frr makefile to handle multiple builds #3483

Fix sonic frr makefile to handle multiple builds #3483

Conversation

nikos-github commented Sep 19, 2019 • edited Loading

pavel-shirshov commented Sep 24, 2019

ben-gale commented Sep 24, 2019 via email

lguohan commented Sep 24, 2019

nikos-github commented Sep 24, 2019

nikos-github commented Sep 24, 2019

sudhanshukumar22 Sep 25, 2019

Choose a reason for hiding this comment

nikos-github Sep 25, 2019

Choose a reason for hiding this comment

nikos-github commented Sep 25, 2019

nikos-github commented Sep 25, 2019

sudhanshukumar22 Sep 25, 2019

Choose a reason for hiding this comment

nikos-github Sep 25, 2019

Choose a reason for hiding this comment

sudhanshukumar22 Sep 25, 2019

Choose a reason for hiding this comment

nikos-github Sep 26, 2019

Choose a reason for hiding this comment

pavel-shirshov left a comment

Choose a reason for hiding this comment

nikos-github commented Sep 27, 2019

pavel-shirshov commented Sep 28, 2019

nikos-github commented Sep 28, 2019 • edited Loading

pavel-shirshov commented Oct 3, 2019

nikos-github commented Sep 19, 2019 •

edited

Loading

nikos-github commented Sep 28, 2019 •

edited

Loading