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

CC15.05: scripts/getver.sh: Fix revision numbering (for Github-based repo) #125

Merged
merged 1 commit into from Oct 23, 2017

Conversation

Projects
None yet
4 participants
@hnyman
Contributor

hnyman commented Oct 11, 2016

Fix revision numbering in the Chaos Calmer branch. CC has been stuck at r49389 since the final move to Github as revision number evaluation has still been based on git-svn-id that is not found in the new original Github commits. So the revision has been stuck at the last svn commit in June.

This patch

  • copies the git revision logic from master and uses the v15.05.1 tag as the base. As the last commit with a known svn revision 49389 was cb4f071 with tag+135, use 49254 as the adjustment. That produces r49461 for the current 8a1f7c9 (and 49389 for cb4f071, as expected).
  • removes the useless svn evaluation (similarly as in master).

Note that this patch also keeps the traditional 'r' prefix (which has accidentally(?) been dropped from master in the last change).

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Oct 17, 2016

Contributor

It would be great to get this merged, or pointers to changes needed, if this approach is not ok. The current brain-dead revision logic in CC serves nobody.

Contributor

hnyman commented Oct 17, 2016

It would be great to get this merged, or pointers to changes needed, if this approach is not ok. The current brain-dead revision logic in CC serves nobody.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Oct 30, 2016

Contributor

ping

Contributor

hnyman commented Oct 30, 2016

ping

@shunjou

This comment has been minimized.

Show comment
Hide comment
@shunjou

shunjou Nov 29, 2016

This is working nicely. It really should get merged already to avoid current CC builds from having revision display stuck at r49389 as mentioned.

shunjou commented Nov 29, 2016

This is working nicely. It really should get merged already to avoid current CC builds from having revision display stuck at r49389 as mentioned.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Nov 30, 2016

Contributor

@wigyori
Any chance to get this merged to fix the CC branch versioning?

Contributor

hnyman commented Nov 30, 2016

@wigyori
Any chance to get this merged to fix the CC branch versioning?

@wigyori

This comment has been minimized.

Show comment
Hide comment
@wigyori

wigyori Dec 1, 2016

Contributor

"Sort of". I'll check it later today, thanks.

Contributor

wigyori commented Dec 1, 2016

"Sort of". I'll check it later today, thanks.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Dec 10, 2016

Contributor

@wigyori

"Sort of". I'll check it later today, thanks.

Please do not forget this.

Contributor

hnyman commented Dec 10, 2016

@wigyori

"Sort of". I'll check it later today, thanks.

Please do not forget this.

@bladeoner

This comment has been minimized.

Show comment
Hide comment
@bladeoner

bladeoner Dec 20, 2016

Hnyman, yesterday I tried to do a pull request for this but what happens is that the getver.sh needs 'Allow execute' rights. That is the reason why the test build in the pull request fails. When I replace the script in my own environment I need to manually put the 'Allow execute' rights on the file in Ubuntu.

Hnyman, yesterday I tried to do a pull request for this but what happens is that the getver.sh needs 'Allow execute' rights. That is the reason why the test build in the pull request fails. When I replace the script in my own environment I need to manually put the 'Allow execute' rights on the file in Ubuntu.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Dec 20, 2016

Contributor

the getver.sh needs 'Allow execute' rights. That is the reason why the test build in the pull request fails.

I don't buy your explanation for the test failure. getver.sh is executable already and this PR just patches a few lines in it without modifying the executable attribute. Also Github shows it as "executable file" also after my patch: https://github.com/hnyman/openwrt/blob/6500b0f769f9bac646e25bcfce9a4689d67898ec/scripts/getver.sh

Downloading and applying the patch preserves the executable status quite properly:

perus@u1610:/Openwrt/github/openwrt$ ls -l scripts/getver.sh 
-rwxr-xr-x 1 perus perus 686 joulu 20 10:28 scripts/getver.sh

perus@u1610:/Openwrt/github/openwrt$ wget https://github.com/hnyman/openwrt/commit/6500b0f769f9bac646e25bcfce9a4689d67898ec.patch
--2016-12-20 10:29:01--  https://github.com/hnyman/openwrt/commit/6500b0f769f9bac646e25bcfce9a4689d67898ec.patch
Resolving github.com (github.com)... 192.30.253.112, 192.30.253.113
...
2016-12-20 10:29:02 (817 KB/s) - ‘6500b0f769f9bac646e25bcfce9a4689d67898ec.patch’ saved [1615]

perus@u1610:/Openwrt/github/openwrt$ patch -p 1 -i 6500b0f769f9bac646e25bcfce9a4689d67898ec.patch 
patching file scripts/getver.sh

perus@u1610:/Openwrt/github/openwrt$ ls -l scripts/getver.sh
-rwxr-xr-x 1 perus perus 698 joulu 20 10:29 scripts/getver.sh

And if you look at the "integration test / check" failure log closely, it seems to fail at the target/install phase, not in the initial phase of base-files build, where getver.sh is used to set the version strings. I have no idea why the test build fails there, but that is likely related to this PR being on CC branch instead of master.

Contributor

hnyman commented Dec 20, 2016

the getver.sh needs 'Allow execute' rights. That is the reason why the test build in the pull request fails.

I don't buy your explanation for the test failure. getver.sh is executable already and this PR just patches a few lines in it without modifying the executable attribute. Also Github shows it as "executable file" also after my patch: https://github.com/hnyman/openwrt/blob/6500b0f769f9bac646e25bcfce9a4689d67898ec/scripts/getver.sh

Downloading and applying the patch preserves the executable status quite properly:

perus@u1610:/Openwrt/github/openwrt$ ls -l scripts/getver.sh 
-rwxr-xr-x 1 perus perus 686 joulu 20 10:28 scripts/getver.sh

perus@u1610:/Openwrt/github/openwrt$ wget https://github.com/hnyman/openwrt/commit/6500b0f769f9bac646e25bcfce9a4689d67898ec.patch
--2016-12-20 10:29:01--  https://github.com/hnyman/openwrt/commit/6500b0f769f9bac646e25bcfce9a4689d67898ec.patch
Resolving github.com (github.com)... 192.30.253.112, 192.30.253.113
...
2016-12-20 10:29:02 (817 KB/s) - ‘6500b0f769f9bac646e25bcfce9a4689d67898ec.patch’ saved [1615]

perus@u1610:/Openwrt/github/openwrt$ patch -p 1 -i 6500b0f769f9bac646e25bcfce9a4689d67898ec.patch 
patching file scripts/getver.sh

perus@u1610:/Openwrt/github/openwrt$ ls -l scripts/getver.sh
-rwxr-xr-x 1 perus perus 698 joulu 20 10:29 scripts/getver.sh

And if you look at the "integration test / check" failure log closely, it seems to fail at the target/install phase, not in the initial phase of base-files build, where getver.sh is used to set the version strings. I have no idea why the test build fails there, but that is likely related to this PR being on CC branch instead of master.

@bladeoner

This comment has been minimized.

Show comment
Hide comment
@bladeoner

bladeoner Dec 20, 2016

Hnyman you are right it doesn't fail at the beginning. Thanks for clarifying. How did you get the file executable? Your patch is extremely useful.

Hnyman you are right it doesn't fail at the beginning. Thanks for clarifying. How did you get the file executable? Your patch is extremely useful.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Dec 20, 2016

Contributor

It is executable already before patching. Like in my example above: it is "-rwxr-xr-x" both before and after patching. If necessary, restore the old pre-patch version with git and then use the exact commands I did above to patch it. (I guess that your editing somehow removes the executable attrib.)

Contributor

hnyman commented Dec 20, 2016

It is executable already before patching. Like in my example above: it is "-rwxr-xr-x" both before and after patching. If necessary, restore the old pre-patch version with git and then use the exact commands I did above to patch it. (I guess that your editing somehow removes the executable attrib.)

scripts/getver.sh: Fix revision numbering (for Github-based repo)
Fix Chaos Calmer revision numbering. CC has been stuck at r49389 since
the final move to Github as revision number evaluation has still been
based on git-svn-id that is not found in the new original Github commits.
So the revision has been stuck at last svn commit in June.

This patch
* copies the git revision logic from master and uses v15.05.1 tag
  as the base. As the last commit with a known svn revision 49389 was
  cb4f071 with tag+135, use 49254 as the adjustment. That produces
  r49461 for the current 8a1f7c9

* removes the useless svn evaluation (similarly as in master).

Signed-off-by: Hannu Nyman <hannu.nyman@iki.fi>
@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Jan 12, 2017

Contributor

@wigyori
Please do not forget this.

Contributor

hnyman commented Jan 12, 2017

@wigyori
Please do not forget this.

@hnyman

This comment has been minimized.

Show comment
Hide comment
@hnyman

hnyman Oct 17, 2017

Contributor

@wigyori
If you are going to compile a CC15.05 release, it might be a good idea to fix the revision numbering first. Otherwise the old SVN based revision number gets applied again.

This PR has been ready for since October 2016, but I have not tested it any more since Dec 2016, when I did the last CC build myself.

Contributor

hnyman commented Oct 17, 2017

@wigyori
If you are going to compile a CC15.05 release, it might be a good idea to fix the revision numbering first. Otherwise the old SVN based revision number gets applied again.

This PR has been ready for since October 2016, but I have not tested it any more since Dec 2016, when I did the last CC build myself.

@wigyori wigyori merged commit 5f0540c into openwrt:chaos_calmer Oct 23, 2017

1 check failed

continuous-integration/drone the build failed
Details

@hnyman hnyman deleted the hnyman:cc_getver branch Oct 23, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment