search

OP-TEE / manifest

Manifests to use OP-TEE on various platforms
69 stars 176 forks source link

Syntax error in stable release #226

Closed gabor-toth-arm closed 1 year ago

gabor-toth-arm commented 2 years ago

Closing backslash is missing from XML entry resulting in repo tool failure in 3.19 (stable) release. See: https://github.com/OP-TEE/manifest/blob/3.19.0/fvp-ts.xml#L9

Correct line would be:

"revision" and "clone-depth" attributes should also be removed from remove-project element. It is not erronous, but does not makes sense IMO.

jbech-linaro commented 2 years ago

@balint-dobszay-arm , can you please have a look into this? Sounds like @gabor-toth-arm is right on this one.

balint-dobszay-arm commented 2 years ago

This only happened for the tagged version, on the master branch it's correct: https://github.com/OP-TEE/manifest/blob/master/fvp-ts.xml#L9

I'm not sure what to do with this, fixing the file and force pushing the tag doesn't sound too good. Maybe a 3.19.1 tag? @jforissier?

jforissier commented 2 years ago

This only happened for the tagged version, on the master branch it's correct: https://github.com/OP-TEE/manifest/blob/master/fvp-ts.xml#L9

The bug is here: https://github.com/OP-TEE/manifest/blob/3.19.0/make_stable.sh#L50 The script wasn't designed to support remove-project, it needs to be fixed.

I'm not sure what to do with this, fixing the file and force pushing the tag doesn't sound too good. Maybe a 3.19.1 tag? @jforissier?

Fine with me. Good way to make sure the (fixed) release script works well :)

balint-dobszay-arm commented 2 years ago

On second thought, the 3.19.1 tag won't work. Using the release script the manifest would refer to a 3.19.1 tag in all OP-TEE repos, which don't exists.

I'm not sure if any people apart from our team are using the fvp-ts.xml. From our perspective just pushing a fix to the 3.19.0 branch and leaving the 3.19.0 tag intact is okay. What do you think?

jbech-linaro commented 2 years ago

What do you think?

Works for me at least.

jforissier commented 2 years ago

What do you think?

Works for me at least.

I'm good with that too.

jenswi-linaro commented 1 year ago

Sounds good to me too.

jforissier commented 1 year ago

Fixed merged. @balint-dobszay-arm would you mind proposing an update to https://github.com/OP-TEE/manifest/blob/3.19.0/make_stable.sh#L50 so that the remove action does not match?

balint-dobszay-arm commented 1 year ago

Sure, I'll create a patch for that.

balint-dobszay-arm commented 1 year ago

Hi @jforissier, Sorry I was procrastinating this for a while, but finally here is the proposed fix: https://github.com/OP-TEE/manifest/pull/230

jforissier commented 1 year ago

@balint-dobszay-arm no worries, this comes in time for the next release! Thank you.