Skip to content
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

autoconf: allow Release to contain hyphen #12437

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

lundman
Copy link
Contributor

@lundman lundman commented Jul 27, 2021

To avoid clashing with tags and releases, we'll use "zfs-macOS".

Motivation and Context

Description

Meta:          1
Name:          zfs-macOS

#define ZFS_META_GITREV "zfs-macOS-2.1.99-85-g872655ba48-dirty"

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Meta:          1
Name:          zfs-macOS

Signed-off-by: Jorgen Lundman <[email protected]>
@lundman lundman mentioned this pull request Jul 27, 2021
13 tasks
@jwk404 jwk404 added the Status: Code Review Needed Ready for review and testing label Jul 28, 2021
@jwk404 jwk404 requested a review from tonyhutter August 30, 2021 18:27
@behlendorf
Copy link
Contributor

I'm a little concerned this might cause some subtle problems since the packaging convention is that hyphens are used to separate the name, version, and release. Allowing a hyphen as part of the name makes it unclear for anything parsing this string where the version and release fields start.

Would it perhaps make more sense to include the "macOS" component in the release field? Is the motivation here to keep the package naming consistent while moving to the upstream code?

@tonyhutter
Copy link
Contributor

> #define ZFS_META_GITREV "zfs-macOS-2.1.99-85-g872655ba48-dirty"

Maybe I'm doing something wrong but I'm not getting that line in my zfs_gitrev.h. Here's what I did:

$ cat META 
Meta:          1
Name:          zfs-macOS
Branch:        1.0
Version:       2.1.99
Release:       1
Release-Tags:  relext
License:       CDDL
Author:        OpenZFS
Linux-Maximum: 5.13
Linux-Minimum: 3.10

$ rm ./include/zfs_gitrev.h
$ ./autogen.sh && ./configure && make -j9
$ cat ./include/zfs_gitrev.h
#define	ZFS_META_GITREV "zfs-2.1.99-397-gbf2032725-dirty"

If I do a make dist I do correctly see the tarball get named zfs-macos-2.1.99.tar.gz though.

@lundman
Copy link
Contributor Author

lundman commented Sep 1, 2021

#define ZFS_META_GITREV "zfs-macOS-2.1.99-85-g872655ba48-dirty"

Apologies - I put that in as an example as to what the version string would look like. To get gitrev to be correct, we simply have to git tag it that way. That is fine, and no change needed.

What it comes down to is three things;

#define ZFS_META_GITREV "zfs-macOS-2.1.99-85-g872655ba48-dirty"

Comes from git tag, no issues.

# sysctl zfs.kext_version
zfs.kext_version: 2.1.99-97_g396215b709

Kernel version string, produced by ZFS_META_VERSION - ZFS_META_RELEASE.
But this is in macOS source files, so we control that - no problem.

Finally;

# zpool version
zfs-macOS-2.1.99-97_g396215b709
zfs-kmod-2.1.99-97_g396215b709

The kernel version comes from macOS sources, and we read it from sysctl. Then it prepends it with "zfs-kmod" - ugly as hell, but small price to be consistent with other platforms. It should probably say kext though.
Userland however, is from ZFS_META_ALIAS which is produced from ZFS_META_ALIAS="$ZFS_META_NAME-$ZFS_META_VERSION"
where $ZFS_META_NAME is from META file and says zfs normally.

If we change $ZFS_META_NAME to say zfs-macOS it fails and just leaves zfs in there.

So we either stop using hyphen in the META file, or autoconf is fixed to be able to handle a hyphen. Since Linux will not use hyphens in META, they should not be affected by this commit (we hope).
Sure we could put zfsMacOS in META but eww.

We need our git tags to be unique to that of Linux (at least for now) since otherwise we'd overlap. Which means the version displayed to the users also need to be reflecting that.

We could also move userland version from common files into _os files, or, #ifdef, or make it based on git tag, instead of META file.

When addressing this issue, we just took the path that seemed to "fix the thing that was broken", but honestly, I don't really mind how it is addressed, it's just cosmetics! :)

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the explanation. Then I think this should be fine.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 2, 2021
@jwk404 jwk404 merged commit 157a4d0 into openzfs:master Sep 2, 2021
rincebrain pushed a commit to rincebrain/zfs that referenced this pull request Sep 22, 2021
To avoid clashing with tags and releases, we'll use "zfs-macOS".

Meta:          1
Name:          zfs-macOS

Reviewed-by: John Kennedy <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#12437
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 15, 2022
To avoid clashing with tags and releases, we'll use "zfs-macOS".

Meta:          1
Name:          zfs-macOS

Reviewed-by: John Kennedy <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#12437
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 16, 2022
To avoid clashing with tags and releases, we'll use "zfs-macOS".

Meta:          1
Name:          zfs-macOS

Reviewed-by: John Kennedy <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#12437
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Feb 17, 2022
To avoid clashing with tags and releases, we'll use "zfs-macOS".

Meta:          1
Name:          zfs-macOS

Reviewed-by: John Kennedy <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Jorgen Lundman <[email protected]>
Closes openzfs#12437
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants