Message ID | 20240111094038.876653-1-kmatsui@gcc.gnu.org |
---|---|
State | Committed |
Commit | df147e2ee7199d33d66959c6509ce9c21072077f |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A3D453857007 for <patchwork@sourceware.org>; Thu, 11 Jan 2024 09:42:28 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A3D453857007 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1704966148; bh=mdaViwuvpXTIW46quw08cXeO19LjoiG7FcZLpxTH6y4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=ZdXGPjfPa3DAUGFTAZ+x9slc72JqcCeBoAUHdLYgNvt3t475jD11iPLTta1rK/f5M LidB7mRARFBWPsj0A/hxZLf0S09F0ftMi+rSximfDmPAqtNhvP8cd1/sHTHSa/LFTJ Mt6TRvD1vuvVyKEjHTM1x+jxrjUDsBEcUSPjh/EM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-00641c01.pphosted.com (mx0a-00641c01.pphosted.com [205.220.165.146]) by sourceware.org (Postfix) with ESMTPS id 68E943857BAC; Thu, 11 Jan 2024 09:41:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 68E943857BAC Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=gcc.gnu.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 68E943857BAC Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=205.220.165.146 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704966079; cv=none; b=YVE1MwdkMyXojFtKN0d3oHbd3DorzvkUGU52sWuK34a/wrcOEI+pAQ67Y6+DnB3+Gw0U2OLKsB+f+qwJr3CGMgpVYHvLdk0WeYfPy5/xumTH0rS301JTiS3Dh7VslCtpTtRj/UbxlQox9GnJuE2jJTkrhOwLycjAZc29xLLHli8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1704966079; c=relaxed/simple; bh=O8WUgy57eOSk/p9oqL3Hu844xadvMd75eC0DwvH7IdI=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=rMPRoOr7JjryGDcMjZg6IwzWHkGqQ4+5eqx8Gq5j4xUcMXLxP/fns4XzhaXPQgMonOBYFDw7dPbpCVnqHfdc/leEly2RnhIxjxTgsGAfBvxKC3bY7K7JkTyCsPdWS3/5Dyv1vtmuN5uo0dBGcu+odIgRzyEMyvXn0bs3zd1WBpk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0247475.ppops.net [127.0.0.1]) by mx0a-00641c01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 40B9UboO028318; Thu, 11 Jan 2024 09:41:13 GMT Received: from mxout23.cac.washington.edu (mxout23.cac.washington.edu [140.142.32.140]) by mx0a-00641c01.pphosted.com (PPS) with ESMTPS id 3vj56y27p7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 11 Jan 2024 09:41:13 +0000 Received: from smtp.washington.edu (smtp.washington.edu [140.142.234.157]) by mxout23.cac.washington.edu (8.14.4+UW20.07/8.14.4+UW22.04) with ESMTP id 40B9enU6032500 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 11 Jan 2024 01:40:49 -0800 X-Auth-Received: from kmatsui-ThinkPad-X1-Carbon-Gen-9.dhcp4.washington.edu ([10.154.74.143]) (authenticated authid=kmatsui) by smtp.washington.edu (8.16.1+UW21.10/8.14.4+UW19.10) with ESMTPSA id 40B9emYd015296 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 11 Jan 2024 01:40:48 -0800 X-UW-Orig-Sender: kmatsui@smtp.washington.edu From: Ken Matsui <kmatsui@gcc.gnu.org> To: gcc-patches@gcc.gnu.org Cc: libstdc++@gcc.gnu.org, Ken Matsui <kmatsui@gcc.gnu.org> Subject: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250] Date: Thu, 11 Jan 2024 01:40:37 -0800 Message-ID: <20240111094038.876653-1-kmatsui@gcc.gnu.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240111062222.525186-1-kmatsui@gcc.gnu.org> References: <20240111062222.525186-1-kmatsui@gcc.gnu.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Proofpoint-GUID: 0tJXjpodAqiFDx4a_pUH2eqEaXTZzoSY X-Proofpoint-ORIG-GUID: 0tJXjpodAqiFDx4a_pUH2eqEaXTZzoSY X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-01-11_04,2024-01-10_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 malwarescore=0 spamscore=0 phishscore=0 mlxlogscore=967 clxscore=1034 lowpriorityscore=0 priorityscore=1501 impostorscore=0 mlxscore=0 adultscore=0 bulkscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000 definitions=main-2401110077 X-Spam-Status: No, score=-12.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NEUTRAL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org |
Series |
[v2,1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]
|
|
Commit Message
Ken Matsui
Jan. 11, 2024, 9:40 a.m. UTC
This patch made std::filesystem::equivalent correctly throw an exception
when either path does not exist as per [fs.op.equivalent]/4.
libstdc++-v3/ChangeLog:
* src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&.
* src/filesystem/ops.cc (fs::equivalent): Likewise.
* testsuite/27_io/filesystem/operations/equivalent.cc: Handle
error codes.
* testsuite/experimental/filesystem/operations/equivalent.cc:
Likewise.
Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
libstdc++-v3/src/c++17/fs_ops.cc | 2 +-
libstdc++-v3/src/filesystem/ops.cc | 2 +-
.../testsuite/27_io/filesystem/operations/equivalent.cc | 4 ++--
.../experimental/filesystem/operations/equivalent.cc | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
Comments
On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > This patch made std::filesystem::equivalent correctly throw an exception > when either path does not exist as per [fs.op.equivalent]/4. Thanks, OK for trunk and all active branches (let me know if you need help backporting it). > > libstdc++-v3/ChangeLog: > > * src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&. > * src/filesystem/ops.cc (fs::equivalent): Likewise. > * testsuite/27_io/filesystem/operations/equivalent.cc: Handle > error codes. > * testsuite/experimental/filesystem/operations/equivalent.cc: > Likewise. > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > --- > libstdc++-v3/src/c++17/fs_ops.cc | 2 +- > libstdc++-v3/src/filesystem/ops.cc | 2 +- > .../testsuite/27_io/filesystem/operations/equivalent.cc | 4 ++-- > .../experimental/filesystem/operations/equivalent.cc | 4 ++-- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc > index e0b308a37ea..61df19753ef 100644 > --- a/libstdc++-v3/src/c++17/fs_ops.cc > +++ b/libstdc++-v3/src/c++17/fs_ops.cc > @@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept > return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; > #endif > } > - else if (!exists(s1) && !exists(s2)) > + else if (!exists(s1) || !exists(s2)) > ec = std::make_error_code(std::errc::no_such_file_or_directory); > else if (err) > ec.assign(err, std::generic_category()); > diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc > index eccdae3d910..4d23a804da0 100644 > --- a/libstdc++-v3/src/filesystem/ops.cc > +++ b/libstdc++-v3/src/filesystem/ops.cc > @@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept > return false; > return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; > } > - else if (!exists(s1) && !exists(s2)) > + else if (!exists(s1) || !exists(s2)) > ec = std::make_error_code(std::errc::no_such_file_or_directory); > else if (err) > ec.assign(err, std::generic_category()); > diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > index 78f6e368204..68f32366d65 100644 > --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > @@ -34,13 +34,13 @@ test01() > bool result; > > result = equivalent(p1, p2, ec); > - VERIFY( ec ); > + VERIFY( ec == std::errc::no_such_file_or_directory ); > VERIFY( !result ); > > __gnu_test::scoped_file f1(p1); > ec = bad_ec; > result = equivalent(p1, p2, ec); > - VERIFY( !ec ); > + VERIFY( ec == std::errc::no_such_file_or_directory ); > VERIFY( !result ); > > __gnu_test::scoped_file f2(p2); > diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > index 929a6ca8609..5bc477a31cd 100644 > --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > @@ -35,13 +35,13 @@ test01() > bool result; > > result = equivalent(p1, p2, ec); > - VERIFY( ec ); > + VERIFY( ec == std::errc::no_such_file_or_directory ); > VERIFY( !result ); > const auto bad_ec = ec; > > __gnu_test::scoped_file f1(p1); > result = equivalent(p1, p2, ec); > - VERIFY( !ec ); > + VERIFY( ec == std::errc::no_such_file_or_directory ); > VERIFY( !result ); > > __gnu_test::scoped_file f2(p2); > -- > 2.43.0 >
On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > This patch made std::filesystem::equivalent correctly throw an exception > > when either path does not exist as per [fs.op.equivalent]/4. > > Thanks, OK for trunk and all active branches (let me know if you need > help backporting it). > Thank you for your review as always! I do not know how to backport this to the active branches. I think the following page is explaining it, but I am not sure how I can know all the active branches. https://gcc.gnu.org/wiki/GitCookbook#backport Do we basically want to git checkout & gcc-backport for each branch after this patch is committed to the trunk? > > > > > libstdc++-v3/ChangeLog: > > > > * src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&. > > * src/filesystem/ops.cc (fs::equivalent): Likewise. > > * testsuite/27_io/filesystem/operations/equivalent.cc: Handle > > error codes. > > * testsuite/experimental/filesystem/operations/equivalent.cc: > > Likewise. > > > > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org> > > --- > > libstdc++-v3/src/c++17/fs_ops.cc | 2 +- > > libstdc++-v3/src/filesystem/ops.cc | 2 +- > > .../testsuite/27_io/filesystem/operations/equivalent.cc | 4 ++-- > > .../experimental/filesystem/operations/equivalent.cc | 4 ++-- > > 4 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc > > index e0b308a37ea..61df19753ef 100644 > > --- a/libstdc++-v3/src/c++17/fs_ops.cc > > +++ b/libstdc++-v3/src/c++17/fs_ops.cc > > @@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept > > return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; > > #endif > > } > > - else if (!exists(s1) && !exists(s2)) > > + else if (!exists(s1) || !exists(s2)) > > ec = std::make_error_code(std::errc::no_such_file_or_directory); > > else if (err) > > ec.assign(err, std::generic_category()); > > diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc > > index eccdae3d910..4d23a804da0 100644 > > --- a/libstdc++-v3/src/filesystem/ops.cc > > +++ b/libstdc++-v3/src/filesystem/ops.cc > > @@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept > > return false; > > return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; > > } > > - else if (!exists(s1) && !exists(s2)) > > + else if (!exists(s1) || !exists(s2)) > > ec = std::make_error_code(std::errc::no_such_file_or_directory); > > else if (err) > > ec.assign(err, std::generic_category()); > > diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > > index 78f6e368204..68f32366d65 100644 > > --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > > +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc > > @@ -34,13 +34,13 @@ test01() > > bool result; > > > > result = equivalent(p1, p2, ec); > > - VERIFY( ec ); > > + VERIFY( ec == std::errc::no_such_file_or_directory ); > > VERIFY( !result ); > > > > __gnu_test::scoped_file f1(p1); > > ec = bad_ec; > > result = equivalent(p1, p2, ec); > > - VERIFY( !ec ); > > + VERIFY( ec == std::errc::no_such_file_or_directory ); > > VERIFY( !result ); > > > > __gnu_test::scoped_file f2(p2); > > diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > > index 929a6ca8609..5bc477a31cd 100644 > > --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > > +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc > > @@ -35,13 +35,13 @@ test01() > > bool result; > > > > result = equivalent(p1, p2, ec); > > - VERIFY( ec ); > > + VERIFY( ec == std::errc::no_such_file_or_directory ); > > VERIFY( !result ); > > const auto bad_ec = ec; > > > > __gnu_test::scoped_file f1(p1); > > result = equivalent(p1, p2, ec); > > - VERIFY( !ec ); > > + VERIFY( ec == std::errc::no_such_file_or_directory ); > > VERIFY( !result ); > > > > __gnu_test::scoped_file f2(p2); > > -- > > 2.43.0 > > >
On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > This patch made std::filesystem::equivalent correctly throw an exception > > > when either path does not exist as per [fs.op.equivalent]/4. > > > > Thanks, OK for trunk and all active branches (let me know if you need > > help backporting it). > > > > Thank you for your review as always! I do not know how to backport this > to the active branches. I think the following page is explaining it, > but I am not sure how I can know all the active branches. > > https://gcc.gnu.org/wiki/GitCookbook#backport Supported releases are listed on the front page at gcc.gnu.org, the active branches are currently releases/gcc-11, releases/gcc-12 and releases/gcc-13. > > Do we basically want to git checkout & gcc-backport for each branch > after this patch is committed to the trunk? Almost. I use gcc-backport for the newest release branch (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit onto gcc-11. The reason for this is that there might be some changes needed on a branch, either to resolve conflicts, or because of other differences on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to backport that to gcc-13 I had to remove the changes to include/bits/version.* and edit include/std/version instead (because we do feature test macros differently on trunk). If I then wanted to backport it to gcc-12 and I just did 'git gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have to resolve the same conflicts again. If I do 'git cherry-pick c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13 branch) then it will apply cleanly to gcc-12, because I'm using the commit that already has the conflicts resolved. Then if I want to backport to gcc-11 as well, use cherry-pick with the hash from the gcc-12 branch. This way any fixes that were needed for branch N-1 will get backported to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit might apply cleanly to every branch. But sometimes the commit needs slightly more massaging to apply to each older branch, so doing it trunk->13 then 13->12 then 12->11 tends to work better. The reason I use cherry-pick after the first backport (instead of gcc-backport every time) is because I don't want a second "(cherry picked from commit ...)" line to be added to the commit message. That's added by gcc-backport (by using cherry-pick -x) but we only need to add it once to be able to track the provenance of the backport, to know which trunk patch was backported. If cherry picking a backport fails and creates a mess of conflicts and you just want to give up and start again, 'git cherry-pick --abort' will undo the changes and leave the working tree clean again. This works whether you use gcc-backport or cherry-pick (because gcc-backport just uses cherry-pick).
On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote: > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > This patch made std::filesystem::equivalent correctly throw an exception > > > > when either path does not exist as per [fs.op.equivalent]/4. > > > > > > Thanks, OK for trunk and all active branches (let me know if you need > > > help backporting it). > > > > > > > Thank you for your review as always! I do not know how to backport this > > to the active branches. I think the following page is explaining it, > > but I am not sure how I can know all the active branches. > > > > https://gcc.gnu.org/wiki/GitCookbook#backport > > Supported releases are listed on the front page at gcc.gnu.org, the > active branches are currently releases/gcc-11, releases/gcc-12 and > releases/gcc-13. > > > > > Do we basically want to git checkout & gcc-backport for each branch > > after this patch is committed to the trunk? > > Almost. I use gcc-backport for the newest release branch > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit > onto gcc-11. > > The reason for this is that there might be some changes needed on a > branch, either to resolve conflicts, or because of other differences > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to > backport that to gcc-13 I had to remove the changes to > include/bits/version.* and edit include/std/version instead (because > we do feature test macros differently on trunk). > > If I then wanted to backport it to gcc-12 and I just did 'git > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have > to resolve the same conflicts again. If I do 'git cherry-pick > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13 > branch) then it will apply cleanly to gcc-12, because I'm using the > commit that already has the conflicts resolved. > > Then if I want to backport to gcc-11 as well, use cherry-pick with the > hash from the gcc-12 branch. > > This way any fixes that were needed for branch N-1 will get backported > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit > might apply cleanly to every branch. But sometimes the commit needs > slightly more massaging to apply to each older branch, so doing it > trunk->13 then 13->12 then 12->11 tends to work better. > > The reason I use cherry-pick after the first backport (instead of > gcc-backport every time) is because I don't want a second "(cherry > picked from commit ...)" line to be added to the commit message. > That's added by gcc-backport (by using cherry-pick -x) but we only > need to add it once to be able to track the provenance of the > backport, to know which trunk patch was backported. > > If cherry picking a backport fails and creates a mess of conflicts and > you just want to give up and start again, 'git cherry-pick --abort' > will undo the changes and leave the working tree clean again. This > works whether you use gcc-backport or cherry-pick (because > gcc-backport just uses cherry-pick). > Thank you for the detailed explanation! I think I was able to backport the patch to the active branches.
On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: > > On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote: > > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > > > This patch made std::filesystem::equivalent correctly throw an exception > > > > > when either path does not exist as per [fs.op.equivalent]/4. > > > > > > > > Thanks, OK for trunk and all active branches (let me know if you need > > > > help backporting it). > > > > > > > > > > Thank you for your review as always! I do not know how to backport this > > > to the active branches. I think the following page is explaining it, > > > but I am not sure how I can know all the active branches. > > > > > > https://gcc.gnu.org/wiki/GitCookbook#backport > > > > Supported releases are listed on the front page at gcc.gnu.org, the > > active branches are currently releases/gcc-11, releases/gcc-12 and > > releases/gcc-13. > > > > > > > > Do we basically want to git checkout & gcc-backport for each branch > > > after this patch is committed to the trunk? > > > > Almost. I use gcc-backport for the newest release branch > > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick > > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit > > onto gcc-11. > > > > The reason for this is that there might be some changes needed on a > > branch, either to resolve conflicts, or because of other differences > > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to > > backport that to gcc-13 I had to remove the changes to > > include/bits/version.* and edit include/std/version instead (because > > we do feature test macros differently on trunk). > > > > If I then wanted to backport it to gcc-12 and I just did 'git > > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have > > to resolve the same conflicts again. If I do 'git cherry-pick > > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13 > > branch) then it will apply cleanly to gcc-12, because I'm using the > > commit that already has the conflicts resolved. > > > > Then if I want to backport to gcc-11 as well, use cherry-pick with the > > hash from the gcc-12 branch. > > > > This way any fixes that were needed for branch N-1 will get backported > > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit > > might apply cleanly to every branch. But sometimes the commit needs > > slightly more massaging to apply to each older branch, so doing it > > trunk->13 then 13->12 then 12->11 tends to work better. > > > > The reason I use cherry-pick after the first backport (instead of > > gcc-backport every time) is because I don't want a second "(cherry > > picked from commit ...)" line to be added to the commit message. > > That's added by gcc-backport (by using cherry-pick -x) but we only > > need to add it once to be able to track the provenance of the > > backport, to know which trunk patch was backported. > > > > If cherry picking a backport fails and creates a mess of conflicts and > > you just want to give up and start again, 'git cherry-pick --abort' > > will undo the changes and leave the working tree clean again. This > > works whether you use gcc-backport or cherry-pick (because > > gcc-backport just uses cherry-pick). > > > > Thank you for the detailed explanation! I think I was able to backport > the patch to the active branches. > For the Bugzilla issue, should I update the status to RESOLVED? Or does someone else handle this? Also, are there other things I should do about this issue? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250 > -- > Ken Matsui
On Thu, 11 Jan 2024 at 12:23, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > > On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote: > > > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > > > > > This patch made std::filesystem::equivalent correctly throw an exception > > > > > > when either path does not exist as per [fs.op.equivalent]/4. > > > > > > > > > > Thanks, OK for trunk and all active branches (let me know if you need > > > > > help backporting it). > > > > > > > > > > > > > Thank you for your review as always! I do not know how to backport this > > > > to the active branches. I think the following page is explaining it, > > > > but I am not sure how I can know all the active branches. > > > > > > > > https://gcc.gnu.org/wiki/GitCookbook#backport > > > > > > Supported releases are listed on the front page at gcc.gnu.org, the > > > active branches are currently releases/gcc-11, releases/gcc-12 and > > > releases/gcc-13. > > > > > > > > > > > Do we basically want to git checkout & gcc-backport for each branch > > > > after this patch is committed to the trunk? > > > > > > Almost. I use gcc-backport for the newest release branch > > > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick > > > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit > > > onto gcc-11. > > > > > > The reason for this is that there might be some changes needed on a > > > branch, either to resolve conflicts, or because of other differences > > > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to > > > backport that to gcc-13 I had to remove the changes to > > > include/bits/version.* and edit include/std/version instead (because > > > we do feature test macros differently on trunk). > > > > > > If I then wanted to backport it to gcc-12 and I just did 'git > > > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have > > > to resolve the same conflicts again. If I do 'git cherry-pick > > > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13 > > > branch) then it will apply cleanly to gcc-12, because I'm using the > > > commit that already has the conflicts resolved. > > > > > > Then if I want to backport to gcc-11 as well, use cherry-pick with the > > > hash from the gcc-12 branch. > > > > > > This way any fixes that were needed for branch N-1 will get backported > > > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit > > > might apply cleanly to every branch. But sometimes the commit needs > > > slightly more massaging to apply to each older branch, so doing it > > > trunk->13 then 13->12 then 12->11 tends to work better. > > > > > > The reason I use cherry-pick after the first backport (instead of > > > gcc-backport every time) is because I don't want a second "(cherry > > > picked from commit ...)" line to be added to the commit message. > > > That's added by gcc-backport (by using cherry-pick -x) but we only > > > need to add it once to be able to track the provenance of the > > > backport, to know which trunk patch was backported. > > > > > > If cherry picking a backport fails and creates a mess of conflicts and > > > you just want to give up and start again, 'git cherry-pick --abort' > > > will undo the changes and leave the working tree clean again. This > > > works whether you use gcc-backport or cherry-pick (because > > > gcc-backport just uses cherry-pick). > > > > > > > Thank you for the detailed explanation! I think I was able to backport > > the patch to the active branches. > > > > For the Bugzilla issue, should I update the status to RESOLVED? Or > does someone else handle this? Also, are there other things I should > do about this issue? > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250 Yes, please close as RESOLVED FIXED and set the target milestone to the first release with the fix, which will be 11.5 In cases like this where the fix is backported to multiple release branches I also add a comment to say "fixed in 11.5, 12.4, 13.3" because otherwise just setting the target milestone to 11.5 doesn't tell you about the other branches. It's also nice to thank the reporter for filing it, especially when they identify the root cause and suggest a fix. And thank you for taking care of it!
On Thu, Jan 11, 2024 at 4:27 AM Jonathan Wakely <jwakely@redhat.com> wrote: > > On Thu, 11 Jan 2024 at 12:23, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote: > > > > > > On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote: > > > > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote: > > > > > > > > > > > > > > This patch made std::filesystem::equivalent correctly throw an exception > > > > > > > when either path does not exist as per [fs.op.equivalent]/4. > > > > > > > > > > > > Thanks, OK for trunk and all active branches (let me know if you need > > > > > > help backporting it). > > > > > > > > > > > > > > > > Thank you for your review as always! I do not know how to backport this > > > > > to the active branches. I think the following page is explaining it, > > > > > but I am not sure how I can know all the active branches. > > > > > > > > > > https://gcc.gnu.org/wiki/GitCookbook#backport > > > > > > > > Supported releases are listed on the front page at gcc.gnu.org, the > > > > active branches are currently releases/gcc-11, releases/gcc-12 and > > > > releases/gcc-13. > > > > > > > > > > > > > > Do we basically want to git checkout & gcc-backport for each branch > > > > > after this patch is committed to the trunk? > > > > > > > > Almost. I use gcc-backport for the newest release branch > > > > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick > > > > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit > > > > onto gcc-11. > > > > > > > > The reason for this is that there might be some changes needed on a > > > > branch, either to resolve conflicts, or because of other differences > > > > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to > > > > backport that to gcc-13 I had to remove the changes to > > > > include/bits/version.* and edit include/std/version instead (because > > > > we do feature test macros differently on trunk). > > > > > > > > If I then wanted to backport it to gcc-12 and I just did 'git > > > > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have > > > > to resolve the same conflicts again. If I do 'git cherry-pick > > > > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13 > > > > branch) then it will apply cleanly to gcc-12, because I'm using the > > > > commit that already has the conflicts resolved. > > > > > > > > Then if I want to backport to gcc-11 as well, use cherry-pick with the > > > > hash from the gcc-12 branch. > > > > > > > > This way any fixes that were needed for branch N-1 will get backported > > > > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit > > > > might apply cleanly to every branch. But sometimes the commit needs > > > > slightly more massaging to apply to each older branch, so doing it > > > > trunk->13 then 13->12 then 12->11 tends to work better. > > > > > > > > The reason I use cherry-pick after the first backport (instead of > > > > gcc-backport every time) is because I don't want a second "(cherry > > > > picked from commit ...)" line to be added to the commit message. > > > > That's added by gcc-backport (by using cherry-pick -x) but we only > > > > need to add it once to be able to track the provenance of the > > > > backport, to know which trunk patch was backported. > > > > > > > > If cherry picking a backport fails and creates a mess of conflicts and > > > > you just want to give up and start again, 'git cherry-pick --abort' > > > > will undo the changes and leave the working tree clean again. This > > > > works whether you use gcc-backport or cherry-pick (because > > > > gcc-backport just uses cherry-pick). > > > > > > > > > > Thank you for the detailed explanation! I think I was able to backport > > > the patch to the active branches. > > > > > > > For the Bugzilla issue, should I update the status to RESOLVED? Or > > does someone else handle this? Also, are there other things I should > > do about this issue? > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250 > > Yes, please close as RESOLVED FIXED and set the target milestone to > the first release with the fix, which will be 11.5 > > In cases like this where the fix is backported to multiple release > branches I also add a comment to say "fixed in 11.5, 12.4, 13.3" > because otherwise just setting the target milestone to 11.5 doesn't > tell you about the other branches. > > It's also nice to thank the reporter for filing it, especially when > they identify the root cause and suggest a fix. > > And thank you for taking care of it! > Done! I truly appreciate your kind step-by-step guidance!
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc index e0b308a37ea..61df19753ef 100644 --- a/libstdc++-v3/src/c++17/fs_ops.cc +++ b/libstdc++-v3/src/c++17/fs_ops.cc @@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; #endif } - else if (!exists(s1) && !exists(s2)) + else if (!exists(s1) || !exists(s2)) ec = std::make_error_code(std::errc::no_such_file_or_directory); else if (err) ec.assign(err, std::generic_category()); diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index eccdae3d910..4d23a804da0 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept return false; return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; } - else if (!exists(s1) && !exists(s2)) + else if (!exists(s1) || !exists(s2)) ec = std::make_error_code(std::errc::no_such_file_or_directory); else if (err) ec.assign(err, std::generic_category()); diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc index 78f6e368204..68f32366d65 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc @@ -34,13 +34,13 @@ test01() bool result; result = equivalent(p1, p2, ec); - VERIFY( ec ); + VERIFY( ec == std::errc::no_such_file_or_directory ); VERIFY( !result ); __gnu_test::scoped_file f1(p1); ec = bad_ec; result = equivalent(p1, p2, ec); - VERIFY( !ec ); + VERIFY( ec == std::errc::no_such_file_or_directory ); VERIFY( !result ); __gnu_test::scoped_file f2(p2); diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc index 929a6ca8609..5bc477a31cd 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc @@ -35,13 +35,13 @@ test01() bool result; result = equivalent(p1, p2, ec); - VERIFY( ec ); + VERIFY( ec == std::errc::no_such_file_or_directory ); VERIFY( !result ); const auto bad_ec = ec; __gnu_test::scoped_file f1(p1); result = equivalent(p1, p2, ec); - VERIFY( !ec ); + VERIFY( ec == std::errc::no_such_file_or_directory ); VERIFY( !result ); __gnu_test::scoped_file f2(p2);