-
Notifications
You must be signed in to change notification settings - Fork 87
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
DISPATCH-2370: Configure "Packit-as-a-Service" application for github.com/apache/qpid-dispatch #1741
base: main
Are you sure you want to change the base?
Conversation
….com/apache/qpid-dispatch
upstream_project_url: https://qpid.apache.org/components/dispatch-router | ||
issue_repository: https://issues.apache.org/jira/projects/DISPATCH/issues | ||
|
||
specfile_path: packaging/qpid-dispatch.spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is aimed at testing changes with fedora-rawhide then the structure should perhaps convey that rather than simply using a root of 'packaging' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was thought out in skupper-router. We wanted to appear vendor neutral. If Ubuntu rolled out it's own Packit-like service, we wanted to have inclusively-named directory where their stuff could also live.
Admit that it would've worked well if there was a single Fedora file, not when there is multiple like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats fine, if its thought all such things can coexist and be easily maintained in one dir. I was just thinking to add a packaging/subdir
for each.
[source,shell script] | ||
---- | ||
dnf install 'dnf-command(builddep)' | ||
dnf builddep -y packaging/skupper-router.spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems questionable
saslMechanisms: ANONYMOUS | ||
} | ||
|
||
+listener { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems unclear that it should have a different default config for this?
Coming back after later looks...is this intended for the console? Unclear if that bit of the spec is even being used? If so and if not, surely this shouldnt be there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how dispatch has been always packaged for Fedora (and also as a Red Hat product). No idea where this came from.
It's true that I removed console from the .spec file. If I can't find a way to build it as part of the spec, I'll revisit this, but if I get the console working, then I'd like to leave this as is.
Previously, the console build has been performed by the .spec file author (on their own machine), and the finished console build was uploaded into fedpkg look aside cache as a "source" file.
# | ||
# Licensed to the Apache Software Foundation (ASF) under one or more | ||
# contributor license agreements. See the NOTICE file distributed with | ||
# this work for additional information regarding copyright ownership. | ||
# The ASF licenses this file to You under the Apache License, Version 2.0 | ||
# (the "License"); you may not use this file except in compliance with | ||
# the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
# Copyright Fedora Project Authors. | ||
# This code is licensed under MIT license |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header and following statements seem in conflict.
Things licenced to the foundation and using the ASF specific header, dont have copyright notices included, and certainly not ones attributing it to another project.
I thought your plan was to write a new file, but if this is just a lift of a Fedora file then it could simply remain MIT and be referenced as such in the LICENCE file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to develop the Fedora .spec in the packaged project itself, and it's continuously kept working as the project evolves. Therefore, when project releases new version, the Fedora package for it is ready to be synced into Fedora and packagers don't have to scramble to adjust to all the changes in the delta from previous packaged release.
This is ofc not relevant for dispatch, but my initial thought was that dispatch will be a proving ground and proton will be where there may be actual benefits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If MIT can be kept throughout, that would be best. Will do it that way.
%undefine __brp_mangle_shebangs | ||
|
||
Name: qpid-dispatch | ||
Version: 3.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm?
Summary: Dispatch router for Qpid | ||
License: ASL 2.0 | ||
URL: http://qpid.apache.org/ | ||
Source0: qpid-dispatch-3.1.0.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm again. More below.
No description provided.