docs/manual: using a branch name as FPP_VERSION does not work

PATCH Superseded Yann E. MORIN 2 participants 3 messages May 10, 2018 – May 11, 2018

Share & tools

Yann E. MORIN

author

2018-05-10 16:43 UTC #0 lore raw

PATCH 1/1 docs/manual: using a branch name as FPP_VERSION does not work

For various reasons, we've always suggested users to avoid using a branch as version string for their packages, because it does not work as a they would expect:

- it is not reproducible, because the branch may change between two

    builds that are done at different times;

- it does not even follow the branch, as Buildroot anyway generates a local tarball.

Yet, until recently, using a branch name would just work (with the above limitations): the git tree was cloned, the branch checked out, and the tarball created.

But with the advent of the git caching, using a branch name does not work anymore. Indeed, we now do a git-fetch, and that does not create local branches. So we can't check out a branch, because it does not exist locally.

Update the manual to state that using a branch does not work. Remove the 'stable' example, as it looked like the name of a stable branch; instead, replace it with a version string that ressemble a tag.

Fix the layout of the manual by making the version examples an actual bulleted list.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Maxime Hadjinlian <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
 docs/manual/adding-packages-generic.txt | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

docs/manual/adding-packages-generic.txt +6 -6

index 7e1f246752..8ea8ce6037 100644
@@ -199,12 +199,12 @@ information is (assuming the package name is +libfoo+) :
 * +LIBFOO_VERSION+, mandatory, must contain the version of the
   package. Note that if +HOST_LIBFOO_VERSION+ doesn't exist, it is
   assumed to be the same as +LIBFOO_VERSION+. It can also be a
-  revision number, branch or tag for packages that are fetched
-  directly from their revision control system. +
-  Examples: +
-    +LIBFOO_VERSION = 0.1.2+ +
-    +LIBFOO_VERSION = cb9d6aa9429e838f0e54faa3d455bcbab5eef057+ +
-    +LIBFOO_VERSION = stable+
+  revision number or a tag for packages that are fetched directly
+  from their revision control system. Do not use a branch name as
+  version; it does not work. Examples:
+  ** a version for a release tarball: +LIBFOO_VERSION = 0.1.2+
+  ** a sha1 for a git tree: +LIBFOO_VERSION = cb9d6aa9429e838f0e54faa3d455bcbab5eef057+
+  ** a tag for a git tree +LIBFOO_VERSION = v0.1.2+
 
 * +LIBFOO_SOURCE+ may contain the name of the tarball of the package,
   which Buildroot will use to download the tarball from

Signature · 1 line

2.14.1

Ricardo Martincoski

2018-05-11 02:59 UTC #1 lore raw

PATCH 1/1 docs/manual: using a branch name as FPP_VERSION does not work

Hello,

TL;DR: I agree with your change to the manual, but I have some nitpick for your explanation.

Quoted 1 line · Yann E. MORIN

docs/manual: using a branch name as FPP_VERSION does not work

s/FPP/FOO/

Quoted 3 lines

For various reasons, we've always suggested users to avoid using a branch as version string for their packages, because it does not work as a they would expect:

OK.

Quoted 2 lines

- it is not reproducible, because the branch may change between two
    builds that are done at different times;

OK.

Quoted 2 lines

- it does not even follow the branch, as Buildroot anyway generates a local tarball.

It does not follow the branch because:
1) Buildroot reuses any local tarball it already generated;
2) with the tarballs for the package removed, when a remote branch was once checked out by the git download infra (by chance, see below) it creates a local branch. When the remote adds a commit to the branch, the download infra fetches the new commits updating origin/branch but the local branch is not updated.

Quoted 6 lines

Yet, until recently, using a branch name would just work (with the above limitations): the git tree was cloned, the branch checked out, and the tarball created.
But with the advent of the git caching, using a branch name does not work anymore.

It does not work ... in a reliable way.
Actually it works *by chance* due to fetch for the special ref. Yes, once again.

I just realized I have to update again my series adding automated tests for the git download infra as it should not test named branches or named feature branches anymore. BTW those tests pass because I did not used 'master'.

If you remove the code for named special refs then the behavior is consistent: a named branch or named feature branch never works.
Well, unless someone is crazy enough to prepend the branch name with 'origin/' in the _VERSION variable. I don't think we should add more complexity to the script to prevent this.

Quoted 3 lines

work anymore. Indeed, we now do a git-fetch, and that does not create local branches. So we can't check out a branch, because it does not exist locally.

Actually the local branch is created on checkout.
With an empty git cache the only branch that does not work is 'master'.

For example, if you first download dl-tools using its current version (a tag) and then change _VERSION to 'master' and download again it works (by chance as describe above).
Starting with an empty git cache and _VERSION set to 'master' does not work because the 'git rev-parse' fails.
Also using an empty git cache and _VERSION set to 'topic/blockdev' does work (again by chance).

Quoted 1 line

Update the manual to state that using a branch does not work. Remove

It does not work for git. CVS branch probably still works (I did not tested). But maybe... we don't care: too much details for the manual. The end game is to discourage the use of branches as it is not good practice, if some kinds of branch works by chance, then OK IMO. But Buildroot does not guarantee that branches work (for simplicity the manual will say it does not work), anyone can still exploit the infra to download a named branch at its own risk, but not for packages in the tree.

Quoted 5 lines

the 'stable' example, as it looked like the name of a stable branch; instead, replace it with a version string that ressemble a tag.
Fix the layout of the manual by making the version examples an actual bulleted list.

By opening the output html in the web browser, I checked the layout is much nicer after this patch.

After all the nitpicking above, I must still say: I am not against applying this patch after fixing when applying only the typo in the title. Anyone can search the mailing list for these details.

Regards,
Ricardo

Yann E. MORIN

author

2018-05-11 16:03 UTC #2 lore raw

PATCH 1/1 docs/manual: using a branch name as FPP_VERSION does not work

Ricardo, All,

On 2018-05-10 23:59 -0300, Ricardo Martincoski spake thusly:

Quoted 2 lines

TL;DR: I agree with your change to the manual, but I have some nitpick for your explanation.

Thanks for the feedback. :-) I've sent an updated version that, I hope, should address all your concerns.

Regards,
Yann E. MORIN.

Quoted 90 lines · Yann E. MORIN

Quoted 2 lines
docs/manual: using a branch name as FPP_VERSION does not work
s/FPP/FOO/
Quoted 3 lines
For various reasons, we've always suggested users to avoid using a branch as version string for their packages, because it does not work as a they would expect:
OK.
Quoted 2 lines
- it is not reproducible, because the branch may change between two
    builds that are done at different times;
OK.
Quoted 2 lines
- it does not even follow the branch, as Buildroot anyway generates a local tarball.
It does not follow the branch because:
1) Buildroot reuses any local tarball it already generated;
2) with the tarballs for the package removed, when a remote branch was once checked out by the git download infra (by chance, see below) it creates a local branch. When the remote adds a commit to the branch, the download infra fetches the new commits updating origin/branch but the local branch is not updated.
Quoted 6 lines
Yet, until recently, using a branch name would just work (with the above limitations): the git tree was cloned, the branch checked out, and the tarball created.
But with the advent of the git caching, using a branch name does not work anymore.
It does not work ... in a reliable way.
Actually it works *by chance* due to fetch for the special ref. Yes, once again.
I just realized I have to update again my series adding automated tests for the git download infra as it should not test named branches or named feature branches anymore. BTW those tests pass because I did not used 'master'.
If you remove the code for named special refs then the behavior is consistent: a named branch or named feature branch never works.
Well, unless someone is crazy enough to prepend the branch name with 'origin/' in the _VERSION variable. I don't think we should add more complexity to the script to prevent this.
Quoted 3 lines
work anymore. Indeed, we now do a git-fetch, and that does not create local branches. So we can't check out a branch, because it does not exist locally.
Actually the local branch is created on checkout.
With an empty git cache the only branch that does not work is 'master'.
For example, if you first download dl-tools using its current version (a tag) and then change _VERSION to 'master' and download again it works (by chance as describe above).
Starting with an empty git cache and _VERSION set to 'master' does not work because the 'git rev-parse' fails.
Also using an empty git cache and _VERSION set to 'topic/blockdev' does work (again by chance).
Quoted 1 line
Update the manual to state that using a branch does not work. Remove
It does not work for git. CVS branch probably still works (I did not tested). But maybe... we don't care: too much details for the manual. The end game is to discourage the use of branches as it is not good practice, if some kinds of branch works by chance, then OK IMO. But Buildroot does not guarantee that branches work (for simplicity the manual will say it does not work), anyone can still exploit the infra to download a named branch at its own risk, but not for packages in the tree.
Quoted 5 lines
the 'stable' example, as it looked like the name of a stable branch; instead, replace it with a version string that ressemble a tag.
Fix the layout of the manual by making the version examples an actual bulleted list.
By opening the output html in the web browser, I checked the layout is much nicer after this patch.
After all the nitpicking above, I must still say: I am not against applying this patch after fixing when applying only the typo in the title. Anyone can search the mailing list for these details.
Regards,
Ricardo

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'