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

xcbuild: 0.1.2-pre -> archived, fix build (x86_64-linux) #123106

Closed
wants to merge 1 commit into from

Conversation

futile
Copy link
Contributor

@futile futile commented May 15, 2021

Update to the last available version as the project has been archived, also fix the build.

Previous failure:

https://hydra.nixos.org/build/142008015/nixlog/2

[...]
FAILED: Libraries/plist/CMakeFiles/plist.dir/Sources/Format/Encoding.cpp.o 
/nix/store/zzvq5qwlm2xikawfqxb0q8gl2bw391a9-gcc-wrapper-10.2.0/bin/g++ -Dplist_EXPORTS -I/nix/store/pj89fimswl2c12dazdfy8cyka7037s6r-libxml2-2.9.10-dev/include/libxml2 -I../Libraries/plist/Headers -I../Libraries/plist/PrivateHeaders -I../Libraries/libutil/Headers -I../Libraries/ext/Headers -std=c++11 -fno-rtti -fno-exceptions -O3 -DNDEBUG -fPIC -Wall -Werror -Wno-multichar -Wno-sign-compare -fdiagnostics-color -MD -MT Libraries/plist/CMakeFiles/plist.dir/Sources/Format/Encoding.cpp.o -MF Libraries/plist/CMakeFiles/plist.dir/Sources/Format/Encoding.cpp.o.d -o Libraries/plist/CMakeFiles/plist.dir/Sources/Format/Encoding.cpp.o -c ../Libraries/plist/Sources/Format/Encoding.cpp
../Libraries/plist/Sources/Format/Encoding.cpp: In static member function 'static std::vector<unsigned char> plist::Format::Encodings::BOM(plist::Format::Encoding)':
../Libraries/plist/Sources/Format/Encoding.cpp:75:5: error: 'abort' was not declared in this scope
   75 |     abort();
      |     ^~~~~
../Libraries/plist/Sources/Format/Encoding.cpp: In function 'Endian EncodingEndian(plist::Format::Encoding)':
../Libraries/plist/Sources/Format/Encoding.cpp:105:5: error: 'abort' was not declared in this scope
  105 |     abort();
      |     ^~~~~
[...]
Motivation for this change

ZHF: #122042

@jonringer (ping meant for @NixOS/nixos-release-managers)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Result of nixpkgs-review run on x86_64-linux 1

4 packages built:
  • xcbuild
  • xcbuild6Hook
  • xcbuildHook
  • xcodebuild6

@futile futile requested a review from matthewbauer as a code owner May 15, 2021 12:56
@futile
Copy link
Contributor Author

futile commented May 15, 2021

@copumpkin @matthewbauer pinging you as you are listed as maintainers.

@futile
Copy link
Contributor Author

futile commented May 15, 2021

Oof, looks like this causes a world-rebuild on darwin. Anything we can do to prevent this, or should I just target staging instead?

Maybe it makes sense to move the import into the #if defined(__linux__) block instead? But abort() is in <cstdlib>, that should also hold true for darwin. I guess it's included through another header on darwin, but not on linux.

Edit: Maybe it makes sense to split this into two (or more) packages, one for linux and one for darwin? That way we could fix the linux build (which doesn't have many reverse-dependencies) independent of the darwin build. Would only be a preventive fix for future issues, as it would still cause a darwin rebuild this time, but at least it would be that.

@r-rmcgibbo
Copy link

Result of nixpkgs-review pr 123106 at c70d6fb5 run on aarch64-linux 1

2 packages built successfully:
  • xcbuild (xcodebuild)
  • xcodebuild6

# we can stop doing this -pre thing.
version = "0.1.2-pre";
# The project has been archived on github, we build the last available revision
version = "archived";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
version = "archived";
version = "unstable-2019-11-20";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that how it's usually done when a git(hub)-repo is archived?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see the need to emphasize the archival status.

Copy link
Member

Choose a reason for hiding this comment

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

Is that how it's usually done when a git(hub)-repo is archived?

Nothing special.

@futile
Copy link
Contributor Author

futile commented May 15, 2021

As I just saw in aliases.nix, other archived projects have instead completely been removed from nixpkgs: https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/aliases.nix#L362

Should we keep this alive, or remove it instead? Don't know what the policy here is.

@jonringer
Copy link
Contributor

Since that was added a year ago, I would just remove it.

Thanks for doing this :)

@jonringer
Copy link
Contributor

@veprbl the package just throws an error for users because of the alias. What would you rather have?

@veprbl
Copy link
Member

veprbl commented May 16, 2021

@jonringer The original question was whether we should remove xcbuild because it may become abandonware, and IMO the general answer should be "it depends". And, in this case we can't remove xcbuild unless we find something to replace, as many things depend on this to build.

@jonringer
Copy link
Contributor

@jonringer The original question was whether we should remove xcbuild because it may become abandonware, and IMO the general answer should be "it depends". And, in this case we can't remove xcbuild unless we find something to replace, as many things depend on this to build.

sure, but anyone on darwin trying to build one of those packages will be met with an error, and that error has been there for the past year

@veprbl
Copy link
Member

veprbl commented May 16, 2021

@jonringer What error are you talking about? It builds just fine.

@jonringer
Copy link
Contributor

apparently these PRs are bleeding together.... there's no error

@veprbl
Copy link
Member

veprbl commented May 16, 2021

apparently these PRs are bleeding together.... there's no error

Actually there is probably still one mentioned in the OP https://hydra.nixos.org/build/142008015, but that is on x86_64-linux (likely because of the gcc10). If the version bump fixes it, we should do it. Otherwise, it's fine to mark xcbuild as broken on Linux.

@futile futile changed the title xcbuild: 0.1.2-pre -> archived, fix build xcbuild: 0.1.2-pre -> archived, fix build (x86_64-linux) May 16, 2021
@futile
Copy link
Contributor Author

futile commented May 16, 2021

Actually there is probably still one mentioned in the OP https://hydra.nixos.org/build/142008015, but that is on x86_64-linux (likely because of the gcc10). If the version bump fixes it, we should do it. Otherwise, it's fine to mark xcbuild as broken on Linux.

Yes, the build I wanted to fix is the x86_64-linux build. The version bump alone does not fix the build, the small patch is also required. If we mark it as broken on linux and don't want to include a patch, we could instead also remove the x86_64-linux version, because the upstream repo is archived, so the build is not expected to be fixed upstream. Otherwise we fix the build for linux with the small patch, and keep the linux version around as well, then we just have to deal with the world-rebuild on darwin.

What's the preferred solution?

@SuperSandro2000
Copy link
Member

/rebase staging

Update to the last available version as the project has been archived,
and fix the build.
@github-actions github-actions bot changed the base branch from master to staging May 16, 2021 23:53
@github-actions github-actions bot closed this May 16, 2021
@github-actions
Copy link
Contributor

Rebased, please reopen the pull request to restart CI

@stale
Copy link

stale bot commented Nov 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@veprbl veprbl closed this Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants