| Message ID | 20211012152410.978077-1-naush@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patches. On Tue, Oct 12, 2021 at 04:24:08PM +0100, Naushir Patuck wrote: > Hi, > > This set of changes have come about after a discusion on an issue raised on the Pi > libcamera-apps github repo [1]. From my little understanding, various distributions > use different methods to build and package libraries. As part of this process, > they may build these libraries outside of the upstream tree, and perhaps not > even in a git repo tree. If this happens, the version string generated during the > libcamera build is either empty or (if it is from a downstream tree) useless. > > This change allows the user to override the SHA value with a string passed into > the meson build options that would be used in-place of the one generated by the > gen-version.sh script. This would allow out-of-tree builds to provide a sensible > sha version string based off the upstream tree. > > I'm not too sure if this is the best way to do this, but it is a simple solution. > If anyone has other suggestions how we can overcome this, please do let me know. We're stepping in the wonderful world of release management :-) We'll certainly need to support builds from release tarballs, that's how most distributions will handle it. We've mostly ignored the issue until now as we have no tagged release, so there was no urgency, but it doesn't mean we have to wait before solving this problem. Once we'll have releases, we will have a version number stored in the top-level meson.build file. The gen-version.sh script will return an empty string, so only the release version will be used. That's been assumed so far to cover all the needs, as if there's an official release, tags and branches in the repository can be used to find the exact commit that was used to generate the tarball. What we haven't considered is builds from tarballs that do not correspond to official releases. In those cases, the binary will report the version number of the last tag, without any sign of local changes. I wonder if we could improve this by automating SHA handling in that case, by automatically adding a file to the tarball when running 'meson dist' with the git commit ID. That file would be read by gen-version.sh if present. That way, generated tarballs will point to a particular commit, and the process should be less error-prone as there won't be any need to set a meson option manually. There are probably a few things details to be figured out (for instance, if the HEAD commit corresponds to a release tag, we likely want to skip inclusion of the SHA1, so maybe the commit ID file should actually be a .version file that stores the output of gen-version.sh), but what do you think of the idea overall ? > [1]: https://github.com/raspberrypi/libcamera-apps/issues/122 > > Naushir Patuck (2): > utils: Add an option to override SHA string in gen-version.sh > build: Add a "version_sha" meson build option > > meson.build | 4 +++- > meson_options.txt | 5 +++++ > src/libcamera/meson.build | 3 ++- > utils/gen-version.sh | 25 ++++++++++++++++++------- > 4 files changed, 28 insertions(+), 9 deletions(-)
On Tue, Oct 12, 2021 at 11:05:00PM +0300, Laurent Pinchart wrote: > Hi Naush, > > Thank you for the patches. > > On Tue, Oct 12, 2021 at 04:24:08PM +0100, Naushir Patuck wrote: > > Hi, > > > > This set of changes have come about after a discusion on an issue raised on the Pi > > libcamera-apps github repo [1]. From my little understanding, various distributions > > use different methods to build and package libraries. As part of this process, > > they may build these libraries outside of the upstream tree, and perhaps not > > even in a git repo tree. If this happens, the version string generated during the > > libcamera build is either empty or (if it is from a downstream tree) useless. > > > > This change allows the user to override the SHA value with a string passed into > > the meson build options that would be used in-place of the one generated by the > > gen-version.sh script. This would allow out-of-tree builds to provide a sensible > > sha version string based off the upstream tree. > > > > I'm not too sure if this is the best way to do this, but it is a simple solution. > > If anyone has other suggestions how we can overcome this, please do let me know. > > We're stepping in the wonderful world of release management :-) > > We'll certainly need to support builds from release tarballs, that's how > most distributions will handle it. We've mostly ignored the issue until > now as we have no tagged release, so there was no urgency, but it > doesn't mean we have to wait before solving this problem. > > Once we'll have releases, we will have a version number stored in the > top-level meson.build file. The gen-version.sh script will return an > empty string, so only the release version will be used. That's been > assumed so far to cover all the needs, as if there's an official > release, tags and branches in the repository can be used to find the > exact commit that was used to generate the tarball. > > What we haven't considered is builds from tarballs that do not > correspond to official releases. In those cases, the binary will report > the version number of the last tag, without any sign of local changes. I > wonder if we could improve this by automating SHA handling in that case, > by automatically adding a file to the tarball when running 'meson dist' > with the git commit ID. That file would be read by gen-version.sh if > present. That way, generated tarballs will point to a particular commit, > and the process should be less error-prone as there won't be any need to > set a meson option manually. > > There are probably a few things details to be figured out (for instance, > if the HEAD commit corresponds to a release tag, we likely want to skip > inclusion of the SHA1, so maybe the commit ID file should actually be a > .version file that stores the output of gen-version.sh), but what do you > think of the idea overall ? This could be implemented with a new script in utils/ that gets run by `meson dist` through meson.add_dist_script() ([1]). [1] https://mesonbuild.com/Reference-manual_builtin_meson.html#mesonadd_dist_script > > [1]: https://github.com/raspberrypi/libcamera-apps/issues/122 > > > > Naushir Patuck (2): > > utils: Add an option to override SHA string in gen-version.sh > > build: Add a "version_sha" meson build option > > > > meson.build | 4 +++- > > meson_options.txt | 5 +++++ > > src/libcamera/meson.build | 3 ++- > > utils/gen-version.sh | 25 ++++++++++++++++++------- > > 4 files changed, 28 insertions(+), 9 deletions(-)
Hi Laurent, On Tue, 12 Oct 2021 at 21:05, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > Thank you for the patches. > > On Tue, Oct 12, 2021 at 04:24:08PM +0100, Naushir Patuck wrote: > > Hi, > > > > This set of changes have come about after a discusion on an issue raised > on the Pi > > libcamera-apps github repo [1]. From my little understanding, various > distributions > > use different methods to build and package libraries. As part of this > process, > > they may build these libraries outside of the upstream tree, and perhaps > not > > even in a git repo tree. If this happens, the version string generated > during the > > libcamera build is either empty or (if it is from a downstream tree) > useless. > > > > This change allows the user to override the SHA value with a string > passed into > > the meson build options that would be used in-place of the one generated > by the > > gen-version.sh script. This would allow out-of-tree builds to provide a > sensible > > sha version string based off the upstream tree. > > > > I'm not too sure if this is the best way to do this, but it is a simple > solution. > > If anyone has other suggestions how we can overcome this, please do let > me know. > > We're stepping in the wonderful world of release management :-) > > We'll certainly need to support builds from release tarballs, that's how > most distributions will handle it. We've mostly ignored the issue until > now as we have no tagged release, so there was no urgency, but it > doesn't mean we have to wait before solving this problem. > > Once we'll have releases, we will have a version number stored in the > top-level meson.build file. The gen-version.sh script will return an > empty string, so only the release version will be used. That's been > assumed so far to cover all the needs, as if there's an official > release, tags and branches in the repository can be used to find the > exact commit that was used to generate the tarball. > > What we haven't considered is builds from tarballs that do not > correspond to official releases. In those cases, the binary will report > the version number of the last tag, without any sign of local changes. This is probably the most likely mode of release for RPi :) I wonder if we could improve this by automating SHA handling in that case, > by automatically adding a file to the tarball when running 'meson dist' > with the git commit ID. That file would be read by gen-version.sh if > present. That way, generated tarballs will point to a particular commit, > and the process should be less error-prone as there won't be any need to > set a meson option manually. > Thanks for the tips! I have read up on meson dist and think I have something that will suit our needs better. In summary, the meson dist will run a script that generates the version string (with utils/gen-version.sh) and saves it into the distro tarball. I then will need to update the top level meson.build to read the version from the version.gen file if it exists, otherwise generate it as normal. The Filesystem modele in meson has a handy fs.read() function, but requires me to bump up the minimum meson version from 0.55 to 0.57 so I will just do a cat and pipe command to read the version string from the file. I will prepare a change and we can discuss it in more detail. Regards, Naush > > There are probably a few things details to be figured out (for instance, > if the HEAD commit corresponds to a release tag, we likely want to skip > inclusion of the SHA1, so maybe the commit ID file should actually be a > .version file that stores the output of gen-version.sh), but what do you > think of the idea overall ? > > > [1]: https://github.com/raspberrypi/libcamera-apps/issues/122 > > > > Naushir Patuck (2): > > utils: Add an option to override SHA string in gen-version.sh > > build: Add a "version_sha" meson build option > > > > meson.build | 4 +++- > > meson_options.txt | 5 +++++ > > src/libcamera/meson.build | 3 ++- > > utils/gen-version.sh | 25 ++++++++++++++++++------- > > 4 files changed, 28 insertions(+), 9 deletions(-) > > -- > Regards, > > Laurent Pinchart >
Hi Naush, On Wed, Oct 13, 2021 at 10:48:17AM +0100, Naushir Patuck wrote: > On Tue, 12 Oct 2021 at 21:05, Laurent Pinchart wrote: > > On Tue, Oct 12, 2021 at 04:24:08PM +0100, Naushir Patuck wrote: > > > Hi, > > > > > > This set of changes have come about after a discusion on an issue raised on the Pi > > > libcamera-apps github repo [1]. From my little understanding, various distributions > > > use different methods to build and package libraries. As part of this process, > > > they may build these libraries outside of the upstream tree, and perhaps not > > > even in a git repo tree. If this happens, the version string generated during the > > > libcamera build is either empty or (if it is from a downstream tree) useless. > > > > > > This change allows the user to override the SHA value with a string passed into > > > the meson build options that would be used in-place of the one generated by the > > > gen-version.sh script. This would allow out-of-tree builds to provide a sensible > > > sha version string based off the upstream tree. > > > > > > I'm not too sure if this is the best way to do this, but it is a simple solution. > > > If anyone has other suggestions how we can overcome this, please do let me know. > > > > We're stepping in the wonderful world of release management :-) > > > > We'll certainly need to support builds from release tarballs, that's how > > most distributions will handle it. We've mostly ignored the issue until > > now as we have no tagged release, so there was no urgency, but it > > doesn't mean we have to wait before solving this problem. > > > > Once we'll have releases, we will have a version number stored in the > > top-level meson.build file. The gen-version.sh script will return an > > empty string, so only the release version will be used. That's been > > assumed so far to cover all the needs, as if there's an official > > release, tags and branches in the repository can be used to find the > > exact commit that was used to generate the tarball. > > > > What we haven't considered is builds from tarballs that do not > > correspond to official releases. In those cases, the binary will report > > the version number of the last tag, without any sign of local changes. > > This is probably the most likely mode of release for RPi :) > > I wonder if we could improve this by automating SHA handling in that case, > > > by automatically adding a file to the tarball when running 'meson dist' > > with the git commit ID. That file would be read by gen-version.sh if > > present. That way, generated tarballs will point to a particular commit, > > and the process should be less error-prone as there won't be any need to > > set a meson option manually. > > Thanks for the tips! I have read up on meson dist and think I have something > that will suit our needs better. In summary, the meson dist will run a script > that generates the version string (with utils/gen-version.sh) and saves it into > the distro tarball. I then will need to update the top level meson.build to > read the version from the version.gen file if it exists, otherwise generate > it as normal. > > The Filesystem modele in meson has a handy fs.read() function, but requires > me to bump up the minimum meson version from 0.55 to 0.57 so I will > just do a cat and pipe command to read the version string from the file. We try not to depend on meson versions that are not available in the latest stable release of the major distributions. In this case, I would thus prefer handling this through a script. Would it make sense to modify gen-version.sh to read the version file when the directory is not under git's control ? > I will prepare a change and we can discuss it in more detail. > > > There are probably a few things details to be figured out (for instance, > > if the HEAD commit corresponds to a release tag, we likely want to skip > > inclusion of the SHA1, so maybe the commit ID file should actually be a > > .version file that stores the output of gen-version.sh), but what do you > > think of the idea overall ? > > > > > [1]: https://github.com/raspberrypi/libcamera-apps/issues/122 > > > > > > Naushir Patuck (2): > > > utils: Add an option to override SHA string in gen-version.sh > > > build: Add a "version_sha" meson build option > > > > > > meson.build | 4 +++- > > > meson_options.txt | 5 +++++ > > > src/libcamera/meson.build | 3 ++- > > > utils/gen-version.sh | 25 ++++++++++++++++++------- > > > 4 files changed, 28 insertions(+), 9 deletions(-)
Hi Laurent, On Wed, 13 Oct 2021 at 11:08, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Naush, > > On Wed, Oct 13, 2021 at 10:48:17AM +0100, Naushir Patuck wrote: > > On Tue, 12 Oct 2021 at 21:05, Laurent Pinchart wrote: > > > On Tue, Oct 12, 2021 at 04:24:08PM +0100, Naushir Patuck wrote: > > > > Hi, > > > > > > > > This set of changes have come about after a discusion on an issue > raised on the Pi > > > > libcamera-apps github repo [1]. From my little understanding, > various distributions > > > > use different methods to build and package libraries. As part of > this process, > > > > they may build these libraries outside of the upstream tree, and > perhaps not > > > > even in a git repo tree. If this happens, the version string > generated during the > > > > libcamera build is either empty or (if it is from a downstream tree) > useless. > > > > > > > > This change allows the user to override the SHA value with a string > passed into > > > > the meson build options that would be used in-place of the one > generated by the > > > > gen-version.sh script. This would allow out-of-tree builds to > provide a sensible > > > > sha version string based off the upstream tree. > > > > > > > > I'm not too sure if this is the best way to do this, but it is a > simple solution. > > > > If anyone has other suggestions how we can overcome this, please do > let me know. > > > > > > We're stepping in the wonderful world of release management :-) > > > > > > We'll certainly need to support builds from release tarballs, that's > how > > > most distributions will handle it. We've mostly ignored the issue until > > > now as we have no tagged release, so there was no urgency, but it > > > doesn't mean we have to wait before solving this problem. > > > > > > Once we'll have releases, we will have a version number stored in the > > > top-level meson.build file. The gen-version.sh script will return an > > > empty string, so only the release version will be used. That's been > > > assumed so far to cover all the needs, as if there's an official > > > release, tags and branches in the repository can be used to find the > > > exact commit that was used to generate the tarball. > > > > > > What we haven't considered is builds from tarballs that do not > > > correspond to official releases. In those cases, the binary will report > > > the version number of the last tag, without any sign of local changes. > > > > This is probably the most likely mode of release for RPi :) > > > > I wonder if we could improve this by automating SHA handling in that > case, > > > > > by automatically adding a file to the tarball when running 'meson dist' > > > with the git commit ID. That file would be read by gen-version.sh if > > > present. That way, generated tarballs will point to a particular > commit, > > > and the process should be less error-prone as there won't be any need > to > > > set a meson option manually. > > > > Thanks for the tips! I have read up on meson dist and think I have > something > > that will suit our needs better. In summary, the meson dist will run a > script > > that generates the version string (with utils/gen-version.sh) and saves > it into > > the distro tarball. I then will need to update the top level > meson.build to > > read the version from the version.gen file if it exists, otherwise > generate > > it as normal. > > > > The Filesystem modele in meson has a handy fs.read() function, but > requires > > me to bump up the minimum meson version from 0.55 to 0.57 so I will > > just do a cat and pipe command to read the version string from the file. > > We try not to depend on meson versions that are not available in the > latest stable release of the major distributions. Agree, I don't want to bump up the version just to use that function. > In this case, I would > thus prefer handling this through a script. Would it make sense to > modify gen-version.sh to read the version file when the directory is not > under git's control ? > This is possible, or I can simply do a run_command('cat ...').stdout() to read the file. I am about to post a RFC patch with the changes in the next few minutes. We can discuss the implementation further there. Regards, Naush > > > I will prepare a change and we can discuss it in more detail. > > > > > There are probably a few things details to be figured out (for > instance, > > > if the HEAD commit corresponds to a release tag, we likely want to skip > > > inclusion of the SHA1, so maybe the commit ID file should actually be a > > > .version file that stores the output of gen-version.sh), but what do > you > > > think of the idea overall ? > > > > > > > [1]: https://github.com/raspberrypi/libcamera-apps/issues/122 > > > > > > > > Naushir Patuck (2): > > > > utils: Add an option to override SHA string in gen-version.sh > > > > build: Add a "version_sha" meson build option > > > > > > > > meson.build | 4 +++- > > > > meson_options.txt | 5 +++++ > > > > src/libcamera/meson.build | 3 ++- > > > > utils/gen-version.sh | 25 ++++++++++++++++++------- > > > > 4 files changed, 28 insertions(+), 9 deletions(-) > > -- > Regards, > > Laurent Pinchart >