Bug 219943 - The '-F' option of ln(1) does not work as expected.
Summary: The '-F' option of ln(1) does not work as expected.
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: amd64 Any
: --- Affects Many People
Assignee: Enji Cooper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-12 15:31 UTC by Shivansh Rai
Modified: 2017-07-17 21:35 UTC (History)
2 users (show)

See Also:
ngie: mfc-stable11+
ngie: mfc-stable10+


Attachments
Log file generated from executing `truss -o ln.log ln -sF A B` (4.62 KB, text/x-log)
2017-06-12 15:31 UTC, Shivansh Rai
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Shivansh Rai 2017-06-12 15:31:12 UTC
Created attachment 183435 [details]
Log file generated from executing `truss -o ln.log ln -sF A B`

I was trying to make a kyua based test for the '-F' option of ln(1).

Steps to reproduce -
This was the command which I ran successfully -
  ```
  mkdir A B
  truss -o ln.log ln -sF A B  # '-f' is assumed to be present by default
  ```

Actual results -
It so happens here that even though the target directory B exists, neither unlink nor rmdir is being called [ideally rmdir should be called as specified in src/bin/ln/ln.c (line 307)].
The output of the above command is that B contains a broken symbolic link A to A.
I used truss(1) to trace the system calls when executing the above command and have attached the log file. As it can be seen in the log file, the rmdir system call is missing.

Expected results -
I think the expected behavior should be that directory B is deleted and a new symbolic link B is made to A.
Comment 1 commit-hook freebsd_committer freebsd_triage 2017-06-12 16:39:12 UTC
A commit references this bug:

Author: ngie
Date: Mon Jun 12 16:38:37 UTC 2017
New revision: 319856
URL: https://svnweb.freebsd.org/changeset/base/319856

Log:
  Add a testcase for `ln -sF`

  The testcase fails today, so mark it with atf_expect_fail: in
  particular, the target (B) isn't being unlinked and the documentation
  doesn't suggest special handling for directories. Thus, there's either
  a doc or an implementation bug in ln(1) that needs to be resolved.

  MFC after:	1 month
  MFC with:	r319714, r319854, r319855
  PR:		219943
  Reviewed by:	ngie
  Submitted by:	shivansh
  Differential Revision:	D11159 (part of a larger diff)
  Sponsored by:	Google, Inc (GSoC 2017)

Changes:
  head/bin/ln/tests/ln_test.sh
Comment 2 Enji Cooper freebsd_committer freebsd_triage 2017-06-12 16:45:37 UTC
Hmmmm... this is contrary to what ln(1) says

139         if (sflag == 0)
140                 Fflag = 0;
141         if (Fflag == 1 && iflag == 0) {
142                 fflag = 1;
143                 wflag = 0;              /* Implied when fflag != 0 */
144         }

     -F    If the target file already exists and is a directory, then remove
           it so that the link may occur.  The -F option should be used with
           either -f or -i options.  If neither -f nor -i is specified, -f is
           implied.  The -F option is a no-op unless -s is specified.
Comment 3 Shivansh Rai 2017-06-12 17:00:29 UTC
(In reply to Ngie Cooper from comment #2)
Aren't the if clauses correct ?

* Fflag is set to zero in the absence of '-s' (hence a no-op).
* In the absence of '-i' and '-f', fflag('-f') is assumed to be present by default.
Comment 4 Enji Cooper freebsd_committer freebsd_triage 2017-06-12 17:29:38 UTC
(In reply to Shivansh Rai from comment #3)

Nevermind -- you're right. The issue's buried further in the code.
Comment 5 commit-hook freebsd_committer freebsd_triage 2017-06-20 20:46:17 UTC
A commit references this bug:

Author: ngie
Date: Tue Jun 20 20:46:08 UTC 2017
New revision: 320172
URL: https://svnweb.freebsd.org/changeset/base/320172

Log:
  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  MFC after:	1 month
  PR:		219943
  Differential Revision:	D11167
  Submitted by:	shivansh
  Sponsored by:	Google (GSoC 2017)

Changes:
  head/bin/ln/ln.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-06-20 20:51:23 UTC
A commit references this bug:

Author: ngie
Date: Tue Jun 20 20:50:54 UTC 2017
New revision: 320173
URL: https://svnweb.freebsd.org/changeset/base/320173

Log:
  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC after:	1 month
  MFC with:	r320172
  PR:		219943
  Differential Revision:	D11167
  Submitted by:	shivansh
  Sponsored by:	Google (GSoC 2017)

Changes:
  head/bin/ln/tests/ln_test.sh
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:09:01 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:08:24 UTC 2017
New revision: 321088
URL: https://svnweb.freebsd.org/changeset/base/321088

Log:
  MFC r319855,r319856,r319858:

  r319855:

  Use readlink(1)/stat(1) to query symlinks instead of file(1)

  file(1) can be compiled out of the system via MK_FILE == no, and the
  output isn't guaranteed to be stable. It's better to use stat(1)/readlink(1)
  instead to query symlink/file paths.

  MFC with:	r319714, r319854

  r319856:

  Add a testcase for `ln -sF`

  The testcase fails today, so mark it with atf_expect_fail: in
  particular, the target (B) isn't being unlinked and the documentation
  doesn't suggest special handling for directories. Thus, there's either
  a doc or an implementation bug in ln(1) that needs to be resolved.

  MFC with:	r319714, r319854, r319855
  PR:		219943

  r319858:

  Style fixes: clean up leading whitespace (8 single column spaces -> \t)

  MFC with:	r319714, r319854, r319855, r319856

Changes:
_U  stable/11/
  stable/11/bin/ln/tests/ln_test.sh
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:12:05 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:11:21 UTC 2017
New revision: 321092
URL: https://svnweb.freebsd.org/changeset/base/321092

Log:
  MFC r320172,r320173:

  r320172:

  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  PR:		219943

  r320173:

  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC with:	r320172
  PR:		219943

Changes:
_U  stable/11/
  stable/11/bin/ln/ln.c
  stable/11/bin/ln/tests/ln_test.sh
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:12:08 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:11:21 UTC 2017
New revision: 321092
URL: https://svnweb.freebsd.org/changeset/base/321092

Log:
  MFC r320172,r320173:

  r320172:

  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  PR:		219943

  r320173:

  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC with:	r320172
  PR:		219943

Changes:
_U  stable/11/
  stable/11/bin/ln/ln.c
  stable/11/bin/ln/tests/ln_test.sh
Comment 10 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:14:15 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:13:44 UTC 2017
New revision: 321094
URL: https://svnweb.freebsd.org/changeset/base/321094

Log:
  MFC r319856,r320172,r320173:

  r319856:

  Add a testcase for `ln -sF`

  The testcase fails today, so mark it with atf_expect_fail: in
  particular, the target (B) isn't being unlinked and the documentation
  doesn't suggest special handling for directories. Thus, there's either
  a doc or an implementation bug in ln(1) that needs to be resolved.

  MFC with:	r319714, r319854, r319855
  PR:		219943

  r320172:

  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  PR:		219943

  r320173:

  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC with:	r320172
  PR:		219943

Changes:
_U  stable/10/
  stable/10/bin/ln/ln.c
  stable/10/bin/ln/tests/ln_test.sh
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:14:17 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:13:43 UTC 2017
New revision: 321094
URL: https://svnweb.freebsd.org/changeset/base/321094

Log:
  MFC r319856,r320172,r320173:

  r319856:

  Add a testcase for `ln -sF`

  The testcase fails today, so mark it with atf_expect_fail: in
  particular, the target (B) isn't being unlinked and the documentation
  doesn't suggest special handling for directories. Thus, there's either
  a doc or an implementation bug in ln(1) that needs to be resolved.

  MFC with:	r319714, r319854, r319855
  PR:		219943

  r320172:

  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  PR:		219943

  r320173:

  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC with:	r320172
  PR:		219943

Changes:
_U  stable/10/
  stable/10/bin/ln/ln.c
  stable/10/bin/ln/tests/ln_test.sh
Comment 12 commit-hook freebsd_committer freebsd_triage 2017-07-17 21:14:19 UTC
A commit references this bug:

Author: ngie
Date: Mon Jul 17 21:13:44 UTC 2017
New revision: 321094
URL: https://svnweb.freebsd.org/changeset/base/321094

Log:
  MFC r319856,r320172,r320173:

  r319856:

  Add a testcase for `ln -sF`

  The testcase fails today, so mark it with atf_expect_fail: in
  particular, the target (B) isn't being unlinked and the documentation
  doesn't suggest special handling for directories. Thus, there's either
  a doc or an implementation bug in ln(1) that needs to be resolved.

  MFC with:	r319714, r319854, r319855
  PR:		219943

  r320172:

  ln(1): fix -F behavior

  When '-F' option is used, the target directory needs to be unlinked.
  Currently, the modified target ("target/source") is being unlinked, and
  since it doesn't yet exist, the original target isn't removed.
  This is fixed by skipping the block where target is modified to
  "target/source" when '-F' option is set.
  Hence, a symbolic link (with the same name as of the original target) to
  the source_file is produced.

  Update the test for ln(1) to reflect fix for option '-F'

  PR:		219943

  r320173:

  Don't expect :sF_flag to fail anymore

  While here, also add a check to verify that the link target
  is updated in the testcase

  MFC with:	r320172
  PR:		219943

Changes:
_U  stable/10/
  stable/10/bin/ln/ln.c
  stable/10/bin/ln/tests/ln_test.sh